Dynamic addition of interfaces in multipath connection

[sanjaybhat2004]

Hello, currently we have a connection between a client and a server setup using picoquic, via multipath. We have modeled our multipath implementation similar to the way it has been done in picoquicdemo.c. However, there the ip address of alternative interface needs to be added by the user at the start of the program manually, while we have written a function, say detect_interfaces() which returns the IP address and interface index of each interface by asking it from the OS. We then iterate through each of these IP, and then if that IP is not the same as the IP address of the primary path, we call picoquic_probe_new_path_ex similar to the way it is done in picoquicdemo.c. This implementation works well, when the required interfaces already exist at the start of the connection.

However, we want to support dynamic addition of interfaces as well, in the sense that if an additional interface comes online during the course of the connection, traffic be routed through it as well. So we come up with following implementation:
(In a mix of C and Pseudo code)

case picoquic_packet_loop_after_receive:

uint64_t mp_cur_probe_timestamp = system_current_time_millis();
if (mp_cur_probe_timestamp - mp_prev_probe_timestamp >= mp_probe_time_period_s * 1000) {

detect_interfaces();

if (ret == 0 && (picoquic_get_cnx_state(cb_ctx->cnx) == picoquic_state_ready ||
            picoquic_get_cnx_state(cb_ctx->cnx) == picoquic_state_client_ready_start)) {

			if (picoquic_get_cnx_state(cb_ctx->cnx) == picoquic_state_ready 
// && cb_ctx->multipath_probe_done == 0
) {

int remote_cid_ready = (picoquic_obtain_stashed_cnxid(cb_ctx->cnx, 1) != NULL);
if (remote_cid_ready) {
	struct sockaddr_storage path0_addr = { 0 };

	if (picoquic_get_path_addr(cb_ctx->cnx, 0, 1, &path0_addr) != 0) {
                        /* This should never happen, as path[0] is always defined before the first migration. */
                    printf("Cannot use multipath. Cannot get address of path 0");
                }

	
	
	for (int i = 0; i < interface_count; i++) {
	
	  if (interface_existed(interface_list[i])) { 
		  // checks if this interface existed in the previous detect_interface call also
		  // if yes, then do not probe that path, as we assume that it is already previous time we came to this callback
		  continue;
	  }
	  
	  if (strcmp(interface_list[i].ipaddr, primary_ip) == 0) {
		  continue;	
	  }	
	  
  
	  if (interface_list[i].alt_ip.ss_family != path0_addr.ss_family) {
                              printf("Cannot add new path, wrong address family, %d vs. %d\n", 
                                  interface_list[i].alt_ip.ss_family, path0_addr.ss_family);
                          }
  
	  else {
		  if ((ret = picoquic_probe_new_path_ex(cb_ctx->cnx, (struct sockaddr*)cb_ctx->server_address_copy,
                                  (struct sockaddr*)&interface_list[i].alt_ip, interface_list[i].index, picoquic_get_quic_time(quic), 0)) != 0) {
                                  printf("Probe new path failed with exit code %d\n", ret);
                              }
                              else {
                                  printf("New path added, total path available %d, new path added is %s\n", cb_ctx->cnx->nb_paths,  interface_list[i].ipaddr);									
                              }


	}


	memcpy(interface_list_copy, interface_list, sizeof(interface_list));
	mp_prev_probe_timestamp = mp_cur_probe_timestamp;
	}
	printf("probe done\n");
	cb_ctx->multipath_probe_done = 1;

}
else {
	printf("remote CID not ready");
}	

}
}

}


break;

However the issue we are encountering is, say there are 2 interfaces initially, and one goes down, and then it comes back. Initially multipath works fine, but later when interface comes back up it actually it does not go into the branch of if statement where probing is there, and instead says "Remote CID not ready". We did not really understand what the get_stashed_cnxid function does, so we were wondering how to do the implementation.

[huitema]

In your example, you have a path failing and being deleted. You then are notified that the address might work again. You cannot reuse the same "path" structure -- once deleted, it cannot be reinstated, and all CID for that old path have been freed. You need to create a new path, with a new path identifier.

Picoquicdemo is using an internal API, picoquic_obtain_stashed_cnxid, which is not really meant for public consumption. That part of the code probably need to be tidied up. We need an API to "get a CID for a new path", and picoquic_obtain_stashed_cnxid is only an approximation of it. Or maybe something like "add new path" that automatically picks the path_id, but that's on the todo list. In between, you need to keep track of the path_ID that have been used already (probably 0 and 1 if you copied picoquicdemo), pick the next value, and call picoquic_obtain_stashed_cnxid with that.

[sanjaybhat2004]

Hello, thank you for you response. Based on your response, we came up with this implementation, which I'm happy to say that it seems to dynamically handle new interfaces as well as old interfaces (even the primary one) going offline and coming online.

For the sake of completeness, and to help the future people who come across this thread, I am adding the implementation here.
Could you also just glance at it and verify that it is not doing anything wrong wrt picoquic library functions?

if (picoquic_get_cnx_state(cb_ctx->cnx) == picoquic_state_ready
    // && cb_ctx->multipath_probe_done == 0
) {
  static int path_cnt = 1;
  int remote_cid_ready =
      (picoquic_obtain_stashed_cnxid(cb_ctx->cnx, path_cnt) != NULL);
  if (remote_cid_ready) {
    detect_interfaces();
    prepare_multipath_config();

    struct sockaddr_storage path0_addr = {0};

    if (picoquic_get_path_addr(cb_ctx->cnx, 0, 1, &path0_addr) != 0) {
      /* This should never happen, as path[0] is always defined before the first
       * migration. */
      printf("Cannot use multipath. Cannot get address of path 0");
    }

    char primary_ip[50];

    inet_ntop(AF_INET, &(((struct sockaddr_in *)&path0_addr)->sin_addr),
              primary_ip, 50);
    printf("primary ip used is: %s\n", primary_ip);

    for (int i = 0; i < interface_count; i++) {

      if (interface_existed(interface_list[i])) {
        // checks if this interface existed in the previous detect_interface
        // if that is the case, do not probe it again 
        continue;
      }

      if (strcmp(interface_list[i].ipaddr, primary_ip) == 0) {
        continue;
      }

      if (path0_addr.ss_family != 0 && /* reason for this check is that once the primary interface is disconnected,
      the primary ip becomes 0.0.0.0 hence if primary interface comes back the probe wont happen*/ 
          interface_list[i].alt_ip.ss_family != path0_addr.ss_family) {
        printf("Cannot add new path, wrong address family, %d vs. %d\n",
               interface_list[i].alt_ip.ss_family, path0_addr.ss_family);
      }

      else {
        if ((ret = picoquic_probe_new_path_ex(
                 cb_ctx->cnx, (struct sockaddr *)cb_ctx->server_address_copy,
                 (struct sockaddr *)&interface_list[i].alt_ip,
                 interface_list[i].index, picoquic_get_quic_time(quic),
                 0)) != 0) {
          printf("Probe new path failed with exit code %d\n", ret);
        } else {
          printf(
              "New path added, total path available %d, new path added is %s\n",
              cb_ctx->cnx->nb_paths, interface_list[i].ipaddr);
        }
      }
    }
    printf("probe done\n");
    cb_ctx->multipath_probe_done = 1;

    interface_count_copy = interface_count;
    memcpy(interface_list_copy, interface_list,
           sizeof(interface_list));
    mp_prev_probe_timestamp = mp_cur_probe_timestamp;

  } else {
    printf("remote CID not ready");
    path_cnt++;
  }
}

[huitema]

You are doing fine with the APIs that you have, but it is a bit sad that you had to go through all that. I think that we need better APIs, see issue #1835

[huitema]

@sanjaybhat2004, I opened issue #1835, "Need API to signal when adding a new path is possible". Please comment on that issue before we settle on a design.

[sanjaybhat2004]

Also I had another concern regarding this, which was related to a issue we were facing.

In our setup as I wrote earlier, we dynamically query the IP of interfaces, and we filter by seeing which are UP and RUNNING, and then pass these IP addresses to the picoqiuc_probe_new_path_ex function. This works fine when there are 2 interfaces connected to the internet, and we are able to have multipath working.

However, we have a case where its failing. The setup was such that, in the IP list, there were 4 interfaces. 1 of them was ethernet which was the primary interface over which the first connection was made (and we avoided probing this interface). The problem arised due to the next 2 interfaces, which were bridge interfaces which had an IP address but were not connected to outside internet. (these might be any type of bridge interfaces, but for the sake of reproducibility it happens in Linux virtual interfaces as well). The next and the 4th interface was a WiFi interface.

Now, what was happening is that, when we looped through this list and called picoquic_probe_new_path_ex on each of these IP's, the return value was 0, signifying that path was probed. However, the cnx->nb_paths variable did not increase, and was stuck on the number 3 for both the 3rd and 4th interface. When we probed the WiFi interface as well the nb_paths variable did not increase. I did a little bit of digging in the code, the place where the behaviour was different than the 2nd path (where nb_paths variable got incremented) was that in the code, after picoquic_create_path is called in picoquic_probe_new_path_ex, in this part:

(quicctx.c file)

uint64_t unique_path_id = picoquic_find_avalaible_unique_path_id(cnx, requested_id);
picoquic_path_t * path_x = (unique_path_id == UINT64_MAX)?NULL:
            (picoquic_path_t *)malloc(sizeof(picoquic_path_t));

the path_x variable was NULL, signifying that the unique_path_id was UINT64_MAX. Trying to understand that function, I came to the conclusion that it is related to stash not having another path.

So 2 things here:

1. I want to understand, what is the reasoning behind returning 0 even when the new path_x variable cannot be allocated and hence a new path cannot be created? (I glanced through the new API and saw that it gives more error info, if this has been fixed there then please ignore this!)
2. I guess the reason that stash does not have another CID is because when the first alternative path is probed, it does not receive any response from the server (since it is not connected to outside internet), please correct me if I'm wrong. So how should we go about sending the probe when such a case happens?

Also all this is using the API at the time when this discussion was created, and not the latest API, we will migrate our code to that in some time.

Also I saw this in the latest documentation:

The error occurs when the peer has not yet provided CIDs for the
*   next path_id. Applications encountering the error should wait until the peer
*   provides CID for the path.  They may want to signal the issue to the peer by
*   queuing a PATH_CIDS_BLOCKED frame.

Could you please provide information on how such a frame might be sent to the server on path 0 so that we can get new CID for probing even if the previous one failed?

Thank for you all the help.

[huitema]

1. I think your issue about "returning 0 even when the new path_x variable cannot be allocated" is fixed in the new API. Please update your code and use that!
2. If probing fails, the path that was used for probing is abandoned. That path number cannot be reused.
3. Attempts to create paths before being authorized by the peer will not succeed. The new API has an event/subscribe process to be notified when new path creation is possible.
4. The code to queue a path blocked frame exists, but the APi is not published yet. That would be:

int picoquic_queue_paths_blocked_frame(
    picoquic_cnx_t* cnx);

5. But sending that frame is purely informational. The peer is not expected to do anything in response.