On a routing vlan, the neighbor entry in the /31 subnet is not added to hardware #771
Conversation
…g added to hardware. Fix: Don't add the net broadcast neighbor entry on the /31 subnet routin vlan as there is no net broadcast address in /31 subnets. Signed-off-by: [email protected]
…g added to hardware. Fix: Don't add the net broadcast neighbor entry on the /31 subnet routin vlan as there is no net broadcast address in /31 subnets. Signed-off-by: [email protected]
…/sonic-swss into subnet_31_neigh_entry
orchagent/intfsorch.cpp
Outdated
| * add a net directed broadcast route specific neighbor entry */ | ||
| if (ip_prefix->getMaskLength() != 31) | ||
| { | ||
| addDirectedBroadcast(port, ip_prefix->getBroadcastIp()); |
There was a problem hiding this comment.
We need to fix getBroadcastIp() and have a check inside addDirectedBroadcast(). We shouldn't be explicitly checking for mask lengths in the code. We just need the APIs to do the right thing.
There was a problem hiding this comment.
I agree in general of APIs doing the right thing. Pushing the /31 checking logic into addDirectedBroadcast(). The getBroadcastIp() just returns broadcast IP irrespective of subnet size. So, i don't think it needs to be changed here.
There was a problem hiding this comment.
Changed with review comments incorporated are submitted.
… addDirectedBroadcast() and removeDirectedBroadcast APIs.
|
Thank you for fixing this. Personally I would also put 2 more checks to make this stricter inside add/remove DirectedBroadcast functions. One that the address is ipv4 and the second one that we return for masks >30 (to include both /31 and /32 just in case). I am approving though and up to you if you want to safeguard this further. |
Changes done. Thanks. |
| @@ -158,9 +158,9 @@ bool IntfsOrch::setIntf(const string& alias, sai_object_id_t vrf_id, const IpPre | |||
| addSubnetRoute(port, *ip_prefix); | |||
| addIp2MeRoute(vrf_id, *ip_prefix); | |||
|
|
|||
| if (port.m_type == Port::VLAN && ip_prefix->isV4()) | |||
There was a problem hiding this comment.
Why this change?. Subnet broadcasting is not supported for IPv6.
orchagent/intfsorch.cpp
Outdated
|
|
||
| /* For /31 and /32 v4 subnets, there is no broadcast address, hence don't | ||
| * add a broadcast route. */ | ||
| if ((ip_prefix.isV4()) && (ip_prefix.getMaskLength() > 30)) |
There was a problem hiding this comment.
Another option is to modify this check as if (!(ip_prefix.isV4()) || (ip_prefix.getMaskLength() > 30)) and edit the comment!.
|
This change has merge conflict with 201811 branch. Cannot be directly cherry-picked. |
…to hardware (sonic-net#771) * In a routing vlan, the neighbor entry in the /31 subnet is not getting added to hardware. Fix: Don't add the net broadcast neighbor entry on the /31 subnet routin vlan as there is no net broadcast address in /31 subnets. Signed-off-by: [email protected]
#784) * On a routing vlan, the neighbor entry in the /31 subnet is not added to hardware (#771) * In a routing vlan, the neighbor entry in the /31 subnet is not getting added to hardware. Fix: Don't add the net broadcast neighbor entry on the /31 subnet routin vlan as there is no net broadcast address in /31 subnets. Signed-off-by: [email protected] * [intfsorch] fix addDirectedBroadcast caller parameter Signed-off-by: Ying Xie <[email protected]>
* Add [show platform pcieinfo] command * Add pcieutil moudle * Add pcietul package * make up for mistake in setup.py * make up the mistake for show/main.py
Signed-off-by: Ze Gan <[email protected]>
What I did
Don't add broadcast ip specific neighbor entry for the /31 subnet routing vlan.
Why I did it
In intfsorch code, it is creating neighbor entry for net directed broadcast address for a Routing VLAN when the routing VLAN interface is created.
In case of /31 subnets there is no broadcast IP as (last 32nd bit) .0 and .1 corresponding to the only 2 hosts in the subnet. When adding neighbor entry for the .1 host, it is conflicting with the neighbor entry created for the directed broadcast IP and hence it throws the below error. As a result the .1 neighbor entry is not added to the hardware.
Dec 8 19:15:55.732017 sonic ERR swss#orchagent: :- meta_sai_validate_neighbor_entry: object key SAI_OBJECT_TYPE_NEIGHBOR_ENTRY:{"ip":"12.12.1.1","rif":"oid:0x6000000000606","switch_id":"oid:0x21000000000000"} already exists
Dec 8 19:15:55.732075 sonic DEBUG swss#orchagent: :< meta_sai_validate_neighbor_entry: exit
Dec 8 19:15:55.732131 sonic DEBUG swss#orchagent: :< meta_sai_create_neighbor_entry: exit
Dec 8 19:15:55.732189 sonic DEBUG swss#orchagent: :< redis_create_neighbor_entry: exit
Dec 8 19:15:55.732245 sonic ERR swss#orchagent: :- addNeighbor: Failed to create neighbor 00:0c:9f:01:02:03 on Vlan900, rv:-6
How I verified it
Have back to back nodes, and configure /31 subnet on both (for eg., 12.1.1.0/31 and 12.1.1.1/31). Ping the /31 peer address (12.1.1.1)
Check that the host entry for 12.1.1.1 is added in the hardware host table.
Details if related