Skip to content

tcp_filters: Remove filter when efrm_filter_redirect fails#282

Open
zhuyifei1999 wants to merge 1 commit intoXilinx-CNS:masterfrom
zhuyifei1999:afxdp-steal-fix
Open

tcp_filters: Remove filter when efrm_filter_redirect fails#282
zhuyifei1999 wants to merge 1 commit intoXilinx-CNS:masterfrom
zhuyifei1999:afxdp-steal-fix

Conversation

@zhuyifei1999
Copy link
Copy Markdown
Contributor

When efrm_filter_redirect fails with ENOENT or ENODEV, the code path would reach efrm_filter_insert, which would try to insert a filter rule with the same filter but different flow steering target queue.

In the case of AF_XDP, efrm_filter_redirect is not implemented (always ENODEV), and some netdevs return an error for af_xdp_filter_insert (via set_rxnfc) when it detects duplicate flow steering rules, even if the target queue is different, requiring the caller to remove the prior filter first. This patch addresses this.

Without the patch, if filter_insert errors, oofilter->filter_id[hwport] is left as the error code from efrm_filter_redirect and the old rule is forgotten by onload and left dangling.

When efrm_filter_redirect fails with ENOENT or ENODEV, the code path
would reach efrm_filter_insert, which would try to insert a filter rule
with the same filter but different flow steering target queue.

In the case of AF_XDP, efrm_filter_redirect is not implemented (always
ENODEV), and some netdevs return an error for af_xdp_filter_insert
(via set_rxnfc) when it detects duplicate flow steering rules,
even if the target queue is different, requiring the caller to remove
the prior filter first. This patch addresses this.

Without the patch, if filter_insert errors, oofilter->filter_id[hwport]
is left as the error code from efrm_filter_redirect and the old rule
is forgotten by onload and left dangling.

Signed-off-by: YiFei Zhu <zhuyifei@google.com>
@sianj-xilinx
Copy link
Copy Markdown
Contributor

Thank you for this suggestion. I'm actually actively working in this area because I'm looking at filter redirect support for one of our other hardware platforms, and had come across this exact issue. I'd like to complete the batch of changes I'm currently looking at and then will come back to this to see whether it still applies as is, or is resolved by the rest of the changes I'm doing.

@mina
Copy link
Copy Markdown

mina commented Jun 3, 2025

Thank you very much. Glad to see we're seeing the same issue. If it's at all possible to give us a preview of the changes you have in mind via a github branch or something, please do. We can help you with the testing.

@mina
Copy link
Copy Markdown

mina commented Sep 12, 2025

Hi sianj-xilinx,

With regards to this bit from your earlier comment:

I'd like to complete the batch of changes I'm currently looking at and then will come back to this to see whether it still applies as is, or is resolved by the rest of the changes I'm doing.

I was wondering if you had a chance to complete your work, and if so, should I retest on a particular onload release where your changes are done/merged? I could refresh this pull request with an updated fix if I still see the issue.

@mina
Copy link
Copy Markdown

mina commented Nov 12, 2025

Hi folks,

This is a very small fix to a critical issue we're encountering with Onload running on AF_XDP. Any update on review merge? What can we do to push this forward?

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