Trap type changes #1258
Trap type changes #1258
Conversation
dkourkouzelis
commented
Jun 28, 2021
dkourkouzelis
commented
- Added UDP port 6784 for BFD and BFDv6 trap types
- Added new LDP trap type
|
please fix error: WARNING: Word 'LDP' is misspelled or is acronym, add to acronyms.txt? saihostif.h 376: * @brief LDP traffic (TCP src port == 646 or TCP dst port == 646) to local |
- Added UDP port 6784 for BFD and BFDv6 trap types - Added new LDP trap type Signed-off-by: Diamantis Kourkouzelis <dkourkou@cisco.com>
Signed-off-by: Diamantis Kourkouzelis <dkourkou@cisco.com>
dkourkouzelis
force-pushed
the
trap-type-changes
branch
from
6efad28 to
a1f21a1
Compare
|
@rck-innovium - please help review/sign off. Thanks. |
inc/saihostif.h
Outdated
| * router IP address (default packet action is drop) | ||
| * @brief BFD traffic (UDP dst port == 3784 or UDP dst port == 4784 or | ||
| * UDP dst port == 6784) to local router IP address (default packet | ||
| * action is drop) |
There was a problem hiding this comment.
Can we add a new trap type like below instead of modifying the existing traps.
Advantages:
-
Does not change behavior for existing implementations
-
Allows optimization of h/w resources when end user needs only BFD and not micro-BFD.
/*
* @brief BFD traffic (UDP dst port == 6784) to local
* router IP address (default packet action is drop)
*/
SAI_HOSTIF_TRAP_TYPE_BFD_MICRO,
Similarly for BFDv6
There was a problem hiding this comment.
I wish you had brought that up during the meeting today. I feel like the rest of the community should be aware of this request given that they all know we decided on using the same trap type.
In regards to your points:
a) I see it differently, given that someone will get BFD over LAG for free pretty much and it's a common use case rather than not.
b)If someone doesn't need BFD over LAG then they don't need to implement it, correct? An entry with type+port will have to be added to the hardware anyway, it's not easy to call out what is being optimized.
Perhaps we need someone to decide whether we need another meeting for this to a) rediscuss with the community, b) go with your proposal or c) go with the original one.
There was a problem hiding this comment.
@rhui Could you please share your thoughts on this?
There was a problem hiding this comment.
It would be good if we can discuss this in the next community meeting.
In most of the TCAM based Host Tables, we would need one ACL entry per L4 port match.
There was a problem hiding this comment.
Made changes per your recommendation.
Signed-off-by: Diamantis Kourkouzelis <dkourkou@cisco.com>
dkourkouzelis
force-pushed
the
trap-type-changes
branch
from
2ca60ef to
d61f4f7
Compare
|
Hi @kcudnik - could you please help with merge, thanks. |
was that already approved on weekly SAI meeting? |
Hi @kcudnik - yes, the PR is reviewed in SAI subgroup meeting. |
Added UDP port 6784 for BFD and BFDv6 trap types Added new LDP trap type
Added UDP port 6784 for BFD and BFDv6 trap types Added new LDP trap type
Added UDP port 6784 for BFD and BFDv6 trap types Added new LDP trap type Co-authored-by: Diamantis Kourkouzelis <dkourkou@cisco.com>
Added UDP port 6784 for BFD and BFDv6 trap types Added new LDP trap type Co-authored-by: Diamantis Kourkouzelis <dkourkou@cisco.com>
|
Hi, with new changes, we have two problems:
|
