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: <vpp-dev-boun...@lists.fd.io> on behalf of Jon Loeliger <j...@netgate.com>
Date: Thursday, 28 September 2017 at 10:20
To: vpp-dev <vpp-dev@lists.fd.io>
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 1 down
TenGigabitEthernet6/0/1 2 down
TenGigabitEthernet6/0/2 3 down
TenGigabitEthernet6/0/3 4 down
local0 0 down
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 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
