Re: [vpp-dev] Adding IP Addr to Multiple IFs
On Fri, Sep 29, 2017 at 3:33 PM, Neale Ranns (nranns) wrote: > Hi Jon, > > > > We don’t support overlapping subnets on interfaces. I was trying to fix > the cases where it is errorneously allowed with this patch: > > https://gerrit.fd.io/r/#/c/8057/ > > but I’ve not yet found all the CSIT failures yet. > > > > /neale > > > Neale, I have submitted a proposed patch for some of the issues here: https://gerrit.fd.io/r/#/c/8619/ The important part of this patch is simply propagating the lower level error message up to the API level where it can be used by the API client. Thanks, jdl ___ vpp-dev mailing list vpp-dev@lists.fd.io https://lists.fd.io/mailman/listinfo/vpp-dev
Re: [vpp-dev] Adding IP Addr to Multiple IFs
Hi Jon, We don’t support overlapping subnets on interfaces. I was trying to fix the cases where it is errorneously allowed with this patch: https://gerrit.fd.io/r/#/c/8057/ but I’ve not yet found all the CSIT failures yet. /neale From: on behalf of Jon Loeliger Date: Thursday, 28 September 2017 at 10:20 To: vpp-dev Subject: [vpp-dev] Adding IP Addr to Multiple IFs Packet Handlers, I have a question regarding adding IP address on multiple interfaces. I know. Seems like it should be obvious and easy. But I am not really a Domain Expert here. So questions about both expected behavior and a possible bug. We'll see. Let's start with this sequence of vppctl commands: vpp# show int Name Idx State Counter Count TenGigabitEthernet6/0/0 1down TenGigabitEthernet6/0/1 2down TenGigabitEthernet6/0/2 3down TenGigabitEthernet6/0/3 4down local00down vpp# set interface ip address TenGigabitEthernet6/0/0 1.2.3.4/32<http://1.2.3.4/32> vpp# set interface ip address TenGigabitEthernet6/0/0 1.2.3.4/32<http://1.2.3.4/32> set interface ip address: failed to add 1.2.3.4/32<http://1.2.3.4/32> which conflicts with 1.2.3.4/32<http://1.2.3.4/32> for interface TenGigabitEthernet6/0/0 So VPP has recognized an overlapping request and complains. Good. But not that it is an exact overlap and could have let it slide. Ah well. vpp# set interface ip address TenGigabitEthernet6/0/1 1.2.3.4/32<http://1.2.3.4/32> vpp# Hrm. No complaint here. Turns out the address is already associated with FIB/table_id 0. OK. But also, no IP address added to the second IF either: vpp# show int address TenGigabitEthernet6/0/0 (dn): 1.2.3.4/32<http://1.2.3.4/32> TenGigabitEthernet6/0/1 (dn): TenGigabitEthernet6/0/2 (dn): TenGigabitEthernet6/0/3 (dn): local0 (dn): Which means it was silently accepted, but didn't do what was asked of it. To be very clear, that means that at the layer above the API call here, there is no way to determine if that API call was successful or not. In fact, that API call always returns "0 == ALL OK" in these cases. So any layer above the API that relied on that return value to store configuration requests in a side DB just got lied to and won't do the Right Thing here. So, digging. I think the first layer problem is in function vl_api_sw_interface_add_del_address_t_handler(): static void vl_api_sw_interface_add_del_address_t_handler (vl_api_sw_interface_add_del_address_t * mp) { vlib_main_t *vm = vlib_get_main (); vl_api_sw_interface_add_del_address_reply_t *rmp; int rv = 0; u32 is_del; VALIDATE_SW_IF_INDEX (mp); is_del = mp->is_add == 0; if (mp->del_all) ip_del_all_interface_addresses (vm, ntohl (mp->sw_if_index)); else if (mp->is_ipv6) ip6_add_del_interface_address (vm, ntohl (mp->sw_if_index), (void *) mp->address, mp->address_length, is_del); else ip4_add_del_interface_address (vm, ntohl (mp->sw_if_index), (void *) mp->address, mp->address_length, is_del); BAD_SW_IF_INDEX_LABEL; REPLY_MACRO (VL_API_SW_INTERFACE_ADD_DEL_ADDRESS_REPLY); } Notice that the REPLY_MACRO() always picks of a return value as rv = 0. So we can add "error = ..." to the add and delete arms, and slam it to 0 in the "delete all" arm, and check for a non-null error to return rv = -1 instead. Adding all that doesn't fix it. (Note that over in ip46_cli.c this is essentially the approach taken there.) Landing in ip4_add_del_interface_address_internal(), the FIB on the sw_if_index is located and the additional IP addr is checked for conflicts with existing entries in the foreach_ip_interface_address() macro loop. That's all fine. It then passes the buck to ip_interface_address_add_del() along with the FIB. The outline of that routine sort of goes like this: addr = lookup IP addr in FIB table check something if (del) delete ip addr set invalid addr index return parameter else if (addr failed to lookup above) add ip addr set new addr index return parameter else silently say we have ip addr already set addr index from lookup in return parameter return success That last "return success" basically means everything will always be happy all the way back to an initial API call essentially every time. So. In the case of adding the same IP address to a second IF, that just happens to have the same (default) FIB on it, I think control flow ends up in the "else" clause above. And that always works. Instead, is there a way to check if the addr is being added to an IF whose sw_if_index is the same-as or different-from the sw_if_index of
[vpp-dev] Adding IP Addr to Multiple IFs
Packet Handlers, I have a question regarding adding IP address on multiple interfaces. I know. Seems like it should be obvious and easy. But I am not really a Domain Expert here. So questions about both expected behavior and a possible bug. We'll see. Let's start with this sequence of vppctl commands: vpp# show int Name Idx State Counter Count TenGigabitEthernet6/0/0 1down TenGigabitEthernet6/0/1 2down TenGigabitEthernet6/0/2 3down TenGigabitEthernet6/0/3 4down local00down vpp# set interface ip address TenGigabitEthernet6/0/0 1.2.3.4/32 vpp# set interface ip address TenGigabitEthernet6/0/0 1.2.3.4/32 set interface ip address: failed to add 1.2.3.4/32 which conflicts with 1.2.3.4/32 for interface TenGigabitEthernet6/0/0 So VPP has recognized an overlapping request and complains. Good. But not that it is an exact overlap and could have let it slide. Ah well. vpp# set interface ip address TenGigabitEthernet6/0/1 1.2.3.4/32 vpp# Hrm. No complaint here. Turns out the address is already associated with FIB/table_id 0. OK. But also, no IP address added to the second IF either: vpp# show int address TenGigabitEthernet6/0/0 (dn): 1.2.3.4/32 TenGigabitEthernet6/0/1 (dn): TenGigabitEthernet6/0/2 (dn): TenGigabitEthernet6/0/3 (dn): local0 (dn): Which means it was silently accepted, but didn't do what was asked of it. To be very clear, that means that at the layer above the API call here, there is no way to determine if that API call was successful or not. In fact, that API call always returns "0 == ALL OK" in these cases. So any layer above the API that relied on that return value to store configuration requests in a side DB just got lied to and won't do the Right Thing here. So, digging. I think the first layer problem is in function vl_api_sw_interface_add_del_address_t_handler(): static void vl_api_sw_interface_add_del_address_t_handler (vl_api_sw_interface_add_del_address_t * mp) { vlib_main_t *vm = vlib_get_main (); vl_api_sw_interface_add_del_address_reply_t *rmp; int rv = 0; u32 is_del; VALIDATE_SW_IF_INDEX (mp); is_del = mp->is_add == 0; if (mp->del_all) ip_del_all_interface_addresses (vm, ntohl (mp->sw_if_index)); else if (mp->is_ipv6) ip6_add_del_interface_address (vm, ntohl (mp->sw_if_index), (void *) mp->address, mp->address_length, is_del); else ip4_add_del_interface_address (vm, ntohl (mp->sw_if_index), (void *) mp->address, mp->address_length, is_del); BAD_SW_IF_INDEX_LABEL; REPLY_MACRO (VL_API_SW_INTERFACE_ADD_DEL_ADDRESS_REPLY); } Notice that the REPLY_MACRO() always picks of a return value as rv = 0. So we can add "error = ..." to the add and delete arms, and slam it to 0 in the "delete all" arm, and check for a non-null error to return rv = -1 instead. Adding all that doesn't fix it. (Note that over in ip46_cli.c this is essentially the approach taken there.) Landing in ip4_add_del_interface_address_internal(), the FIB on the sw_if_index is located and the additional IP addr is checked for conflicts with existing entries in the foreach_ip_interface_address() macro loop. That's all fine. It then passes the buck to ip_interface_address_add_del() along with the FIB. The outline of that routine sort of goes like this: addr = lookup IP addr in FIB table check something if (del) delete ip addr set invalid addr index return parameter else if (addr failed to lookup above) add ip addr set new addr index return parameter else silently say we have ip addr already set addr index from lookup in return parameter return success That last "return success" basically means everything will always be happy all the way back to an initial API call essentially every time. So. In the case of adding the same IP address to a second IF, that just happens to have the same (default) FIB on it, I think control flow ends up in the "else" clause above. And that always works. Instead, is there a way to check if the addr is being added to an IF whose sw_if_index is the same-as or different-from the sw_if_index of the original addr in the FIB entry? Could we return "all happy" if they match, and return some error in the case of non-matching? Does that make any sense and make sense to do, if it is even possible? Or am I out in the weeds again? Thanks, jdl ___ vpp-dev mailing list vpp-dev@lists.fd.io https://lists.fd.io/mailman/listinfo/vpp-dev