On 17/10/2019 17:22, Pablo Neira Ayuso wrote:
> You refer to single u32 word changing both sport and dport.
>
> What's the point with including this in the subset of the uAPI to be
> supported?
What's the point with removing this from the uAPI which implicitly the
kernel internal layers supported (whether individual drivers did or
not)?
> In software, it really makes sense to use this representation since it
> is speeding up packet processing.
>
> In hardware, supporting this uAPI really gets you nothing at all.
That depends how the hardware works. Current hardware tends to do pedit
offload based on protocol(-ossified) fields, but there's no guarantee
that every future piece of hardware will be the same. Someone might
build HW whose packet-editing part is based on the pedit uAPI approach,
explicitly designing their HW to support Linux semantics. And you'd be
telling them "sorry, we decided to remove part of that uAPI because,
uhh, well we didn't actually have a reason but we did it anyway".
> We have to document what subset of the uAPI is support through
> hardware offloads. Pretending that we can support all uAPI is not
> true, we only support for tc pedit extended mode right now.
Who's "we"? AIUI the kernel will pass any pedits to drivers, they don't
have to be extended mode, and they certainly don't have to have come
from the iproute2 'tc' binary, so they might not bear any relation to
the protocol fields tc knows about.
Pedit is supposed to work even in the absence of protocol knowledge in
the kernel (e.g. in combination with cls_u32, you can filter and mangle
traffic in a completely new protocol), you're turning it into Yet
Another Ossified TCP/IP Monoculture. This is not the direction the
networking offloads community is trying to move in.
+100 to everything Jakub said, and please remember that some of us have
to maintain driver support for slightly more than just the latest
kernel, and your pointless churn makes that much harder. ("A slow sort
of country; here, it takes all the running _you_ can do, just to stay
in the same place.") I'm not talking about drivers stretching back to
2.6 era here; we _expect_ that to be painful. But when code to build
on as recent as 4.20 is a thicket of clustered ifdefs, without a single
piece of user-visible functionality being added as a result (and some
removed; not only are you chopping bits of the pedit API off, but TC
action stats are *still* broken since 5.1), something is _very_ wrong.
Of course we know the real reason you're making all these API changes is
for netfilter offload (I have my doubts whether anyone is actually
using netfilter in the wild; crusties still use iptables and early-
adopters have all jumped ship to eBPF solutions, but if you're so
desperate for netfilter to remain relevant I suppose we have to humour
you at least a little), but there's absolutely no technical reason I
can see why netfilter couldn't translate its mangles into existing
pedit language. If patch 3 is truly and unavoidably a prerequisite of
patch 4, you'll need to explain why if you want a sympathetic hearing.
-Ed
