sonic-sairedis icon indicating copy to clipboard operation
sonic-sairedis copied to clipboard

Published 20 hours ago • verified publisher icon sonic-net

[syncd] Use hash as queue for fdb notifications

Open kcudnik opened this issue 3 years ago • 4 comments

This will drop multiple fdb notifications in case of flood but will preserve last one

kcudnik avatar Jun 08 '22 16:06 kcudnik

this could drop the aging event, what would be the impact to the orchagent.

lguohan avatar Jun 13 '22 14:06 lguohan

i'm not sure if OA is ready for dropped events, like dropping AGE event, we would need to discuss with @prsunny

kcudnik avatar Jun 13 '22 14:06 kcudnik

Currently the MAC move storm looks as following:

  1. leant on port A
  2. aged from port A
  3. learnt on port B
  4. aged from port B
  5. learnt from port A
  6. aged from port A
  7. learnt from port B
  8. Aged from port B
  9. Learnt from port A

My understanding is that if the last event is: "9", then it will be treated as NOP since MAC was already learnt on port A. So in between all the moves were ignored as the start and end are the same. Not sure if any protocol has a need to know MAC did move but end up at the same port at then end. probably not a big issue for this scenario. "8", I think the current code does not pay attention to the port and will go ahead honor the Age and remove the MAC. which if that is the case it should also be fine as the final state for this MAC should be removed from MAC table. "7", I think the current code would ignore this new learnt event to port B since the MAC was already learnt on port A. But MAC should be on port B. So this will post an issue... "6", I think this would be fine. MAC would be removed correctly. "5", This is similar to step "9" which we already covered earlier.

So, in all scenarios where the MAC move storm ended at step "7" would be the issue that needs to be solved. Also, I do not think that the MAC checking does not involve port check. it only checks for that given VLAN... I think this is not correct but not sure if this is something we need to address it now or leave it at a future PR improvement... but fixing case ending at "7" may require to validate MAC event on both VLAN and Port... @prsunny please help confirm if my listed scenarios are correct...

gechiang avatar Jun 20 '22 23:06 gechiang

agree with @gechiang that currently orchagent does not handle learn event on another port. From the current implementation, other cases are expected to be handled.

prsunny avatar Jun 21 '22 00:06 prsunny