Add support for application filtering in UP4 plugin #410
Conversation
|
CC: @daniele-moro |
| tableID := t.tableID(TableApplications) | ||
| entry := &p4.TableEntry{ | ||
| TableId: tableID, | ||
| // priority for UP4 cannot be greater than 65535 |
There was a problem hiding this comment.
Where does this information come from? Priority in p4rt is int32. This may be a problem because precedence is a uint32.
There was a problem hiding this comment.
However, priority in ROC is from 0 (low) to 250 (high). We need to understand how precedence is calculated to do our assumption here.
There was a problem hiding this comment.
this is the outcome from STC testing. The UP4 app rejected table entry with the following message: Priority cannot be greater than 65535. I see that MAX_PRIORITY = 65535 is used among different ONOS modules: https://github.com/opennetworkinglab/onos/blob/2b4de873e4033b7973b399d25cb8828a73bc2e24/core/api/src/main/java/org/onosproject/net/flow/FlowRule.java#L32. May it be the reason?
There was a problem hiding this comment.
Yes, definitely that's the reason, thanks for the explanation!
There was a problem hiding this comment.
It would be safer to check if precedence <= 65535 and return an error if not (maybe in an utility function shared with other translation functions, since we use ternary in different tables). Also, limiting the table entry priority to 255 (MaxUint8) is too conservative without any benefit. I would consider only the ONOS limit for now.
There was a problem hiding this comment.
@ccascone I'm going to merge this as is and add the precedence verification in a separate PR.
| tableID := t.tableID(TableApplications) | ||
| entry := &p4.TableEntry{ | ||
| TableId: tableID, | ||
| // priority for UP4 cannot be greater than 65535 |
There was a problem hiding this comment.
It would be safer to check if precedence <= 65535 and return an error if not (maybe in an utility function shared with other translation functions, since we use ternary in different tables). Also, limiting the table entry priority to 255 (MaxUint8) is too conservative without any benefit. I would consider only the ONOS limit for now.
|
|
||
| appProto, appProtoMask := pdr.appFilter.proto, pdr.appFilter.protoMask | ||
|
|
||
| appIPPrefixLen := 32 - bits.TrailingZeros32(appIPMask) |
There was a problem hiding this comment.
Is there any guarantee that the mask is equivalent to an LPM? Can there be other zeros besides the trailing ones?
There was a problem hiding this comment.
This is guaranteed by the format of SDF Filter that uses IP/prefixlen.
Co-authored-by: Carmelo Cascone <carmelo@opennetworking.org>
|
retest this please |
|
retest this please |
1 similar comment
|
retest this please |
This PR adds support fro application filtering in UP4 plugin.
Depends on: #432