Hi Aleksander, > The xt_flowoffload module is inconditionally setting on the hardware > offload flag: [...] > > which is triggering the slow down because packet path is allocating > work to offload the entry to hardware, however, this driver does not > support for hardware offload. > > Probably this module can be updated to unset the flowtable flag if the > harware does not support hardware offload. yesterday there was a discussion about this on the #openwrt-devel IRC channel. I am adding the IRC log to the end of this email because I am not sure if you're using IRC. I typically don't test with flow offloading enabled (I am testing with OpenWrt's "default" network configuration, where flow offloading is disabled by default). Also I am not familiar with the flow offloading code at all and reading the xt_FLOWOFFLOAD code just raised more questions for me. Maybe you can share some info whether your workaround from [0] "fixes" this issue. I am aware that it will probably break other devices. But maybe it helps Pablo to confirm whether it's really an xt_FLOWOFFLOAD bug or rather some generic flow offload issue (as Felix suggested on IRC). Best regards, Martin [0] https://github.com/abajk/openwrt/commit/ee4d790574c0edd170e1710d7cd4819727b23721 <rsalvaterra> nbd: I saw your flow offloading updates. Just to make sure, this issue hasn't been addressed yet, has it? https://lore.kernel.org/netdev/20210614215351.GA734@salvia/ <nbd> i don't think so <nbd> can you reproduce it? <rsalvaterra> nbd: Not really, I don't have the hardware. <rsalvaterra> It's lantiq, I think (bthh5a). <rsalvaterra> However, I believe dwmw2_gone has one, iirc. <xdarklight> nbd: I also have a HH5A. if you have any patch ready you can also send it as RFC on the mailing list and Cc Aleksander <rsalvaterra> xdarklight: Have you been able to reproduce the flow offloading regression? <xdarklight> I can try (typically I test with a "default" network configuration, where flow offloading is disabled) <rsalvaterra> xdarklight: I don't have a lot of details, only this thread: https://github.com/openwrt/openwrt/pull/4225#issuecomment-855454607 <xdarklight> rsalvaterra: this is the workaround that Aleksander has in his tree: https://github.com/abajk/openwrt/commit/ee4d790574c0edd170e1710d7cd4819727b23721 <rsalvaterra> xdarklight: Well, but that basically breaks hw flow offloading everywhere else, if I'm reading correctly. :) <xdarklight> rsalvaterra: I am not arguing with that :). I wanted to point out that Pablo's finding on the netfilter mailing list seems to be correct <rsalvaterra> xdarklight: Sure, which is why I pinged nbd, since he's the original author of the xt_FLOWOFFLOAD target. <rsalvaterra> What it seems is that it isn't such trivial fix. :) <xdarklight> I looked at the xt_FLOWOFFLOAD code myself and it raised more questions than I had before looking at the code. so I decided to wait for someone with better knowledge to look into that issue :) <rsalvaterra> Same here. I just went "oh, this requires divine intervention" and set it aside. :P <nbd> xdarklight: which finding did you mean? <xdarklight> nbd: "The xt_flowoffload module is inconditionally setting on the hardware offload flag" [...] flowtable[1].ft.flags = NF_FLOWTABLE_HW_OFFLOAD; <xdarklight> nbd: from this email: https://lore.kernel.org/netdev/20210614215351.GA734@salvia/ <nbd> i actually think that finding is wrong <nbd> xt_FLOWOFFLOAD registers two flowtables <nbd> one with hw offload, one without <nbd> the target code does this: <nbd> table = &flowtable[!!(info->flags & XT_FLOWOFFLOAD_HW)]; <nbd> so it selects flowtable[1] only if info->flags has XT_FLOWOFFLOAD_HW set <rsalvaterra> nbd: That's between you and Pablo, I mustn't interfere. :) <nbd> i did reply to pablo, but never heard back from him <rsalvaterra> nbd: The merge window is still open, he's probably busy at the moment… maybe ping him on Monday? <xdarklight> nbd: it seems that your mail also didn't make it to the netdev mailing list (at least I can't find it) <rsalvaterra> xdarklight: Now that you mention it, neither do I. <nbd> he wrote to me in private for some reason <xdarklight> oh okay. so for me to summarize: you're saying that the xt_FLOWOFFLOAD code should be fine. in that case Aleksander's workaround (https://github.com/abajk/openwrt/commit/ee4d790574c0edd170e1710d7cd4819727b23721) is also a no-op and the original problem would still be seen <rsalvaterra> xdarklight: I don't think it's a no-op, otherwise he wouldn't carry it in his tree… maybe something's wrong in the table selection? table = &flowtable[!!(info->flags & XT_FLOWOFFLOAD_HW)]; does look correct, though. <nbd> xdarklight: well, it's not a no-op if the issue was reproduced with option flow_offloading_hw 1 in the config <rsalvaterra> nbd: Uh… If he has option flow_offloading_hw '1' on a system which doesn't support hw flow offloading… he gets to keep the pieces, right? <nbd> it shouldn't break <nbd> and normally i'd expect the generic flow offload api to be able to deal with it without attempting to re-enable hw offload over and over again
