Skip to content
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

support session AMBR & MFBR & URR MBQE(measurement before QoS enforcement) #86

Merged
merged 18 commits into from
Feb 16, 2024

Conversation

chen042531
Copy link
Contributor

No description provided.

@chen042531 chen042531 force-pushed the sessAMBR branch 2 times, most recently from 87849ce to aecd472 Compare December 25, 2023 01:13
src/gtpu/encap.c Outdated Show resolved Hide resolved
src/genl/genl_qer.c Outdated Show resolved Hide resolved
src/genl/genl_qer.c Outdated Show resolved Hide resolved
src/genl/genl_pdr.c Outdated Show resolved Hide resolved
src/genl/genl_qer.c Outdated Show resolved Hide resolved
src/gtpu/trTCM.c Show resolved Hide resolved
src/gtpu/encap.c Outdated Show resolved Hide resolved
src/gtpu/encap.c Outdated Show resolved Hide resolved
src/proc.c Outdated Show resolved Hide resolved
src/proc.c Outdated Show resolved Hide resolved
src/gtpu/encap.c Outdated
@@ -774,7 +788,7 @@ static int gtp5g_fwd_skb_encap(struct sk_buff *skb, struct net_device *dev,
uh->check = 0;

if (pdr->urr_num != 0) {
ret = check_urr(pdr, far, volume, volume_mbqe, true);
ret = check_urr(pdr, far, volume, volume_mbqe, true, drop_pkt);
if (ret < 0) {
if (ret == DONT_SEND_UL_PACKET) {
GTP5G_ERR(pdr->dev, "Should not foward the first uplink packet");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this GTP5G_ERR?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chen042531 please reply.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unsure why the original author used GTP5G_ERR()
I believe it would be better to use GTP5G_INF() at level 3 instead, as this is not an error.

src/gtpu/encap.c Outdated Show resolved Hide resolved

if (color == Red){
return -1;
}
ret = netif_rx(skb);
if (ret != NET_RX_SUCCESS) {
GTP5G_ERR(dev, "Uplink: Packet got dropped\n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we return value other than 0?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chen042531 please reply.

Copy link
Contributor

@afcidk afcidk Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are still mixed usages for 0, -1, COLOR_RED_DROP.
I think COLOR_RED_DROP should be value other than 0 (maybe -2?) so that we can distinguish different runtime behaviors (forwarded, dropped due to what failed reason, dropped due to QoS).

By the way, related error codes may need to be updated as well

gtp5g/src/gtpu/encap.c

Lines 143 to 152 in 49fbe75

case 1:
GTP5G_ERR(gtp->dev, "Pass up to the process\n");
break;
case 0:
break;
case -1:
GTP5G_ERR(gtp->dev, "GTP packet has been dropped\n");
kfree_skb(skb);
ret = 0;
break;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will update these part

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed with @chen042531, only 1, 0, -1 should be returned from encap function. The return values will be replaced with macro, and only return value with -1 (dropped) should be freed.

src/gtpu/encap.c Outdated Show resolved Hide resolved
@chen042531 chen042531 force-pushed the sessAMBR branch 2 times, most recently from d522421 to 4b1954c Compare January 9, 2024 10:09
src/gtpu/encap.c Outdated Show resolved Hide resolved
src/gtpu/encap.c Outdated Show resolved Hide resolved
@chen042531 chen042531 force-pushed the sessAMBR branch 2 times, most recently from a37e780 to d27cde2 Compare January 12, 2024 01:41
include/pdr.h Outdated Show resolved Hide resolved
include/qer.h Outdated Show resolved Hide resolved
src/genl/genl_pdr.c Outdated Show resolved Hide resolved
src/genl/genl_qer.c Outdated Show resolved Hide resolved
src/gtpu/encap.c Outdated Show resolved Hide resolved
src/gtpu/encap.c Outdated Show resolved Hide resolved
@chen042531 chen042531 changed the title session AMBR v1 support session AMBR & MFBR & URR MBQE(measurement before QoS enforcement) Jan 24, 2024
src/gtpu/encap.c Outdated Show resolved Hide resolved
src/gtpu/encap.c Outdated Show resolved Hide resolved
tim-ywliu
tim-ywliu previously approved these changes Feb 5, 2024
@tim-ywliu tim-ywliu dismissed their stale review February 5, 2024 16:52

some comments unresolved

if (color == Red) {
GTP5G_TRC(pdr->dev, "Drop red packet");
return PKT_DROPPED;
}
ret = netif_rx(skb);
if (ret != NET_RX_SUCCESS) {
GTP5G_ERR(dev, "Uplink: Packet got dropped\n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to free the skb when netif_rx fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

”netif_rx“ will free the skb when netif_rx fails

include/encap.h Outdated
@@ -6,6 +6,10 @@
#include "dev.h"
#include "pktinfo.h"

#define PKT_PASS_TO_UPF 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are the difference between PKT_PASS_TO_UPF and PKT_SENT? Does it mean to send pkt to APP or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"PKT_PASS_TO_UPF" means that gtp5g cannot handle the packet (e.g., not GTPv1, not TPDU, or not an end marker) and passes the packet to a userspace program for handling.

"PKT_SENT" indicates that gtp5g has successfully handled the packet and forwarded it to the N6/N9 interface.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to PKT_TO_APP and PKT_FORWARDED

src/gtpu/encap.c Outdated
if (ret < 0) {
if (ret == DONT_SEND_UL_PACKET) {
GTP5G_ERR(pdr->dev, "Should not foward the first uplink packet");
GTP5G_INF(pdr->dev, "Should not foward the first uplink packet");
return -1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return PKT_DROPPED;

src/gtpu/encap.c Outdated
if (color == Red) {
GTP5G_TRC(pdr->dev, "Drop red packet");
return PKT_DROPPED;
}
if (ip_xmit(skb, pdr->sk, dev) < 0) {
GTP5G_ERR(dev, "Failed to transmit skb through ip_xmit\n");
return -1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return PKT_DROPPED;

src/gtpu/encap.c Outdated
if (check_urr(pdr, far, volume, volume_mbqe, true) < 0)
GTP5G_ERR(pdr->dev, "Fail to send Usage Report");
}

return 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return PKT_FORWARDED;

@tim-ywliu tim-ywliu merged commit 7768779 into free5gc:master Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants