-
Notifications
You must be signed in to change notification settings - Fork 639
SMF-UPF set interface proto definitions #2033 #2437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: ronit-kumar-acl <ronit.k@altencalsoftlabs.com>
| Action action = 2; | ||
| } | ||
|
|
||
| message FlowMatchNew { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs IP protocol field.
| uint32 tcp_dst = 4; | ||
| uint32 udp_src = 5; | ||
| uint32 udp_dst = 6; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for tcp vs udp port, just have one set for l4 src and dst port.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure , I'll take care of the same.
| bytes mac_dst_value = 2; | ||
| bytes upper_mac_src_value = 3; | ||
| bytes upper_mac_dst_value = 4; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you define what is upper mac value means?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upper Source MAC address value contains the upper value of an MAC address range.
8.2.93 MAC address
https://www.etsi.org/deliver/etsi_ts/129200_129299/129244/15.08.00_60/ts_129244v150800p.pdf
|
|
||
|
|
||
|
|
||
| message SdfFilters { //8.2.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include link to reference doc in source code file. This would make it easier to find referenced section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'll include the below in src file too.
https://www.etsi.org/deliver/etsi_ts/129200_129299/129244/15.08.00_60/ts_129244v150800p.pdf
| DENY = 1; | ||
| } | ||
| Action action = 2; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you define single set of action, msg FlowDescriptor and ApplyAction both has action and already diverged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think SetGroupFAR dictates the forwarding action so lets just remove this Action from here
|
|
||
| message FlowMatchNew { | ||
| string ipv4_src = 1; | ||
| string ipv4_dst = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the IP address shld be able ip version agnostic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi
Could you please elaborate the "able ip version agnostic". I didn't understand this.
| message EthPackFilter { | ||
| uint32 eth_filt_id = 1; | ||
| uint32 eth_filt_prop = 2; | ||
| MacAddress mac_addr = 3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which end point has this mac address ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PDI IE within PFCP Session Establishment Request has this as an IE.
7.5.2.2-3: Ethernet Packet Filter IE within PFCP Session Establishment Request
https://www.etsi.org/deliver/etsi_ts/129200_129299/129244/15.08.00_60/ts_129244v150800p.pdf
| uint32 s_tag = 5; | ||
| SdfFilters sdf_filters = 6; | ||
| uint32 ethertype = 7; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we going to support ethernet filtering, we need to support otherL2 related stuff in session establishment (5.13.1).
I do not see that in this this protobuf, Am I missing something?
| //UserPlId user_pl_id = 11; | ||
| //UserId user_id = 12; | ||
| //TraceInfo tr_info = 13; | ||
| //ApnDnn apn_dnn = 14; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its not good practice to keep commented out code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'l take care of the same.
|
|
||
|
|
||
| message UpfRes {} | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove void msgs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it has been now updated and will be pushed soon,
| uint32 tcp_dst = 4; | ||
| uint32 udp_src = 5; | ||
| uint32 udp_dst = 6; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may consider Oneof type here: https://developers.google.com/protocol-buffers/docs/proto3#oneof
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll check and modify.
| //PDR message Table 7.5.2.2-1: Create PDR IE within PFCP Session Establishment Request | ||
| message SetGroupPDR { | ||
| uint32 pdr_id = 1; | ||
| int32 sess_ver_no = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this sess_ver_no refer to? Is this the same value as the session level version in SessionSet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a discussion which was going on wherein the group messages will also maintain a version number apart from the Set message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a discussion which was going on wherein the group messages will also maintain a version number apart from the Set message.
We should remove it, confirmed with @pshelar. Keeping one version number is going to be simpler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, thanks.
| uint32 bar_id = 6; | ||
| } | ||
|
|
||
| // ToDO SetGroupQER write |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add something like our current QOS policy here?
message FlowQos {
enum Qci {
QCI_0 = 0;
QCI_1 = 1;
QCI_2 = 2;
QCI_3 = 3;
QCI_4 = 4;
QCI_5 = 5;
QCI_6 = 6;
QCI_7 = 7;
QCI_8 = 8;
QCI_9 = 9;
QCI_65 = 65;
QCI_66 = 66;
QCI_67 = 67;
QCI_70 = 70;
QCI_75 = 75;
QCI_79 = 79;
}
uint32 max_req_bw_ul = 1;
uint32 max_req_bw_dl = 2;
uint32 gbr_ul = 3;
uint32 gbr_dl = 4;
Qci qci = 5;
QosArp arp = 6;
}
| } | ||
|
|
||
|
|
||
| message ApplyAction { //8.2.26 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we can simplify this structure a bit by just defining a enum of Action and then directly using the repeated Action.
For example:
enum Action {
DROP = 0;
FORW = 1;
...
}
message SetGroupFAR {
uint32 far_id = 1; //FAR ID should be less than 2^32
...
repeated Action action_to_apply = 3;
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Marie, I will modify accordingly and push.
|
Sorry for delay in response, somehow I didn't receive the notification or I missed it. I'll get back with responses. |
|
#2540 Commit contains the updated pipelined.proto along with the client to talk to pipelined. |
|
Closing as the latest proto have been defined in #3268. |
Signed-off-by: ronit-kumar-acl ronit.k@altencalsoftlabs.com
Summary
Common set interface proto definitions for 5G. Contains SMF to UPF Proto definitions.
Test Plan
Compiled successfully inside AGW VM.
Additional Information