Re: [PATCH net-next] net: dsa: Remove indirect function call for flow dissection

[Florian Fainelli]

On 1/3/20 1:28 PM, Vladimir Oltean wrote:
> On Fri, 3 Jan 2020 at 22:50, Florian Fainelli <f.fainelli@xxxxxxxxx> 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