lkml.org 
[lkml]   [2021]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH nf] Revert "netfilter: flowtable: Remove redundant hw refresh bit"
From
Date
Hi Martin,

Thanks for the IRC log. Today I repeated my previous tests. I think I had to have Hardware flow offloading enabled before, even though Lantiq xRX200 doesn't support it. With only the software flow offloading turned on, I do not see a significant performance drop.

Today's results (IPv6 routing with DSA driver and Burst Length Patch):
Device Kernel Flow offload Upload Download
HH5A 5.4.128 Disabled 101 170
HH5A 5.10.46 Disabled 95.4 159
HH5A 5.10.42 Disabled 96.6 165
HH5A 5.10.41 Disabled 101 165
HH5A 5.4.128 Soft 471 463
HH5A 5.10.46 Soft 553 472
HH5A 5.10.42 Soft 556 468
HH5A 5.10.41 Soft 558 492
HH5A 5.4.128 Soft+Hard 434 460
HH5A 5.10.46 Soft+Hard 229 181
HH5A 5.10.42 Soft+Hard 228 177
HH5A 5.10.41 Soft+Hard 577 482

It seems my workaround is unnecessary.

Best regards,
Aleksander Jan Bajkowski

On 7/11/21 3:02 AM, Martin Blumenstingl wrote:
> 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
>

\
 
 \ /
  Last update: 2021-07-11 14:33    [W:0.976 / U:0.376 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site