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

eBPF helper function for attribute search in the netlink message #178

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kalimuthu-Velappan
Copy link

@Kalimuthu-Velappan Kalimuthu-Velappan commented Dec 4, 2020

There are few network applications relying on Netlink subsystem to get
notifications for net-device attribute changes like MTU, Speed,
Oper-Status, Name, slave, slave info, etc. The Netlink subsystem
notifies the application on every attribute change regardless of what
is needed for the application. The attribute search support in
EBPF filter helps to filter the Netlink packets based on the specific
set of attributes that are needed for the application.

The classical BPF supports attribute search but that doesn't support
MAPS. The extended BPF supports MAPS, but the attribute search is not
enabled. Hence this patch enables the support for attribute search in
EBPF.

This patch adds the support for following helper function.
FN(skb_get_nlattr),
FN(skb_get_nlattr_nest)

skb_get_nlattr:
Find a specific attribute in a stream of attributes

skb_get_nlattr_nest:
Find a specific attribute in a stream of nested attributes

@lguohan
Copy link
Contributor

lguohan commented Dec 5, 2020

is this upstream patch? if not, can you upstream this patch first?

Copy link
Contributor

@paulmenzel paulmenzel left a comment

Choose a reason for hiding this comment

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

is this upstream patch? if not, can you upstream this patch first?

I couldn’t find it in Linus’ Linux master branch v5.10-rc6-178-gb3298500b23f. So yes, please send it upstream first.

Can you also give a small example, how to use the new helper functions? Please also not on what device you tested this.

@@ -0,0 +1,70 @@
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use a git format-patch formatted patch, which can be easily applied with git am ….patch to the Linux kernel source. Please add a descriptive commit summary, message and Signed-off-by line.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_ANYTHING,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please align the equal signs with tabs as is done in the rest of the file.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_ANYTHING,
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

@Kalimuthu-Velappan
Copy link
Author

is this upstream patch? if not, can you upstream this patch first?

We have already upstreamed this to the Linux community. But it is taking time to merge.
https://www.spinics.net/lists/netdev/msg627990.html

I will address your comments and resubmit the patch agagin.

@lguohan
Copy link
Contributor

lguohan commented Dec 10, 2020

i looked the thread, the question is reasonable, we will need to wait till that one gets signed off.

@Kalimuthu-Velappan
Copy link
Author

Fixed the alignment issue and added the descriptive summary.

The PR 6131 provides the library support on how to use these attributes.
sonic-net/sonic-buildimage#6131

@ben-gale
Copy link

@Kalimuthu-Velappan , @lguohan , @paulmenzel - this one has been quiet for a few days - can we get it moving towards merge pls?

@lguohan - I'm not sure I understand your comment saying "i looked the thread, the question is reasonable, we will need to wait till that one gets signed off" - which thread and which question are you talking about please?

@paulmenzel
Copy link
Contributor

Could you please rebase it on the master branch? Currently already committed changes are present in the commit list.

@Kalimuthu-Velappan
Copy link
Author

retest this please

There are few network applications relying on Netlink subsystem to get
notifications for net-device attribute changes like MTU, Speed,
Oper-Status, Name, slave, slave info, etc. The Netlink subsystem
notifies the application on every attribute change regardless of what
is needed for the application. The attribute search support in
EBPF filter helps to filter the Netlink packets based on the specific
set of attributes that are needed for the application.

The classical BPF supports attribute search but that doesn't support
MAPS. The extended BPF supports MAPS, but the attribute search is not
enabled. Hence this patch enables the support for attribute search in
EBPF.

This patch adds the support for following helper function.
    FN(skb_get_nlattr),
    FN(skb_get_nlattr_nest)

skb_get_nlattr:
    Find a specific attribute in a stream of attributes

skb_get_nlattr_nest:
    Find a specific attribute in a stream of nested attributes
@jarias-lfx
Copy link

/easycla

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants