[meta] Flush fdb entries after flush api success#581
[meta] Flush fdb entries after flush api success#581qiluo-msft merged 3 commits intosonic-net:masterfrom
Conversation
meta/Meta.cpp
Outdated
| } | ||
|
|
||
| if (fdbTypeAttr->getSaiAttr()->value.s32 != type->value.s32) | ||
| if ((type->value.s32 != SAI_FDB_FLUSH_ENTRY_TYPE_ALL) && |
There was a problem hiding this comment.
How about this?
if (type->value.s32 == SAI_FDB_FLUSH_ENTRY_TYPE_ALL) {
// add fdb to remove vector and continue;
}
// check type match
// check bridge_port_id matches
// check vlan ID matches
If you dont like this, then when someone specified type == SAI_FDB_FLUSH_ENTRY_TYPE_ALL, and also provides vlan_id or bridge_port_id or both, we are NOT going to flush all. Is that the expected behavior? Or we should honor the type attribute?
There was a problem hiding this comment.
Also, what do we print on line number 6963 when value is SAI_FDB_FLUSH_ENTRY_TYPE_ALL? Because we care casting from FLUSH_ENTRY_TYPE to FDB_ENTRY_TYPE enum
There was a problem hiding this comment.
cant be like that , since if it's ALL (static and dynamic, other attributes also needs to be checked, like vlan id, or bridge port, ALL here appllies only to static or dynamic
There was a problem hiding this comment.
Got it. SAI_FDB_FLUSH_ENTRY_TYPE_ALL means BOTH(STATIC and DYNAMIC) type! NOT really ALL FDB entries!
There was a problem hiding this comment.
there are no other entries except static and dynamic, so ALL means static and dynamic together
meta/Meta.cpp
Outdated
| list[1].value.s32 = et ? et->value.s32 : SAI_FDB_FLUSH_ENTRY_TYPE_ALL; | ||
|
|
||
| data.event_type = SAI_FDB_EVENT_FLUSHED; | ||
| data.fdb_entry.switch_id = switch_id; |
There was a problem hiding this comment.
Should we have fdb_flush_entry as well in sai_fdb_event_notification_data_t and use it when event is SAI_FDB_EVENT_FLUSHED. Otherwise, we are converting FDB_FLUSH_ENTRY attributes to FDB_ENTRY attributes and in-fact if you look at the second attribute here, attribute type is SAI_FDB_ENTRY_ATTR_TYPE and value is SAI_FDB_FLUSH_ENTRY_TYPE_ALL. what do you say?
There was a problem hiding this comment.
yes, already addressed in below commit
| auto *et = sai_metadata_get_attr_by_id(SAI_FDB_FLUSH_ATTR_ENTRY_TYPE, attr_count, attr_list); | ||
|
|
||
| sai_attribute_t list[2]; | ||
| if (et) |
There was a problem hiding this comment.
What is someone specifies the type but sets it to TYPE_ALL?
There was a problem hiding this comment.
already fixed in my next commit
|
@kcudnik Can we backport this to 201911 branch? |
* [meta] Flush fdb entries after flush api success * [meta] Use correct fdb entry type * [meta] Use correct enum values for fdb entry
I did it. @kcudnik or @qiluo-msft could you please approve and merge. |
* [meta] Flush fdb entries after flush api success * [meta] Use correct fdb entry type * [meta] Use correct enum values for fdb entry
* [meta] Flush fdb entries after flush api success * [meta] Use correct fdb entry type * [meta] Use correct enum values for fdb entry Co-authored-by: Kamil Cudnik <[email protected]>
…ic-net#585) * [meta] Flush fdb entries after flush api success * [meta] Use correct fdb entry type * [meta] Use correct enum values for fdb entry Co-authored-by: Kamil Cudnik <[email protected]>
…ic-net#585) (#5) * [meta] Flush fdb entries after flush api success * [meta] Use correct fdb entry type * [meta] Use correct enum values for fdb entry Co-authored-by: Kamil Cudnik <[email protected]> Co-authored-by: Kamil Cudnik <[email protected]>
* [meta] Flush fdb entries after flush api success * [meta] Use correct fdb entry type * [meta] Use correct enum values for fdb entry
4 participants
No description provided.