Messages in this thread | | | Subject | Re: [PATCH net-next] net: dsa: Remove indirect function call for flow dissection | From | Florian Fainelli <> | Date | Wed, 8 Jan 2020 10:03:36 -0800 |
| |
On 1/3/20 1:28 PM, Vladimir Oltean wrote: > On Fri, 3 Jan 2020 at 22:50, Florian Fainelli <f.fainelli@gmail.com> wrote: >> >> The call path is the following on TX (e.g.: when you run a DHCP client), > > Oh, it gets called on TX too, ok. > In that case, static proto_off information won't work for asymmetric > taggers such as ocelot which may have an independently configurable > prefix length on RX and TX. > I want to get rid of the RX tag prefix in ocelot though, but just saying. > >> I don't think your formula works for EDSA which has an EtherType, but > > Why doesn't it work with edsa?
It would, my bad.
> >> this would probably work for all tags we currently support except trailer. >> >> proto = (__be16 *)(skb->data)[overhead / 2 - 1]; >> > > I wasn't suggesting to do this exact calculation in flow_dissector.c, > but rather to pre-populate proto_off with a value statically derived > from it on a piece of paper, with the trailer exception where it would > be -2 in bytes or -1 in shorts, but nonetheless a negative and valid > value.
With the trailer, the EtherType is actually at the expected location, that is 12 bytes from the beginning of the Ethernet frame, so we can simplify things even more.
> >> >> I don't think anyone except Alexander did serious investigation this. >> For now, what I am interested in is reducing the amount of technical >> debt and expensive function calls. > > Does the change bring any measurable improvement?
I did not implement flow dissection for Broadcom tags largely because it did not show up as a performance problem with the different customers but I will try to collect some numbers. At any rate, the patch is not meant to be a performance improvement (though it might provide some improvements) but ease maintenance and make it more straight forward for future protocols to automatically gain dissection without having to provide a function pointer. -- Florian
| |