Messages in this thread |  | | From | Vladimir Oltean <> | Date | Fri, 3 Jan 2020 02:19:58 +0200 | Subject | Re: [PATCH net-next] net: dsa: Remove indirect function call for flow dissection |
| |
Hi Florian,
On Fri, 3 Jan 2020 at 01:39, Florian Fainelli <f.fainelli@gmail.com> wrote: > > We only need "static" information to be given for DSA flow dissection, > so replace the expensive call to .flow_dissect() with an integer giving > us the offset into the packet array of bytes that we must de-reference
packet array? packed array?
> to obtain the protocol number. The overhead was alreayd available from
already
> the dsa_device_ops structure so use that directly. > > The presence of a flow_dissect callback used to indicate that the DSA > tagger supported returning that information,we now encode this with a > proto_off value of DSA_PROTO_OFF_UNPSEC if the tagger does not support
UNSPEC
> providing that information yet. > > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > ---
Unfortunately I don't really understand the DSA implementations of flow_dissect. Is proto_off supposed to mean "the __be16 pointer difference A - B between A. the position of the real EtherType and B. the current skb->data (aka ETH_HLEN bytes into the frame, aka 2 bytes after the normal EtherType was supposed to be)"? Otherwise said, the offset in bytes between the real EtherType position and skb->data is 2 * (proto_off + 1). Furthermore, the offset in bytes is exactly equal to the tagger overhead in bytes, unless it's a tag that doesn't push the EtherType to the right, such as the trailer tag.
If the above is indeed correct, can you just skip DSA_PROTO_OFF_UNSPEC and add proper proto_off values "in blind" for all taggers? I think it's rather safe to assume that they all push the EtherType to the right with the exception of the trailer tag, which will have an offset of -1 in terms of __be16 pointers, by the way (so your -1 encoding of DSA_PROTO_OFF_UNSPEC won't work for it anyway).
Also, documenting the unit of measurement for proto_off would really go a long way.
What is a good test that the flow_dissector does what it's supposed to do with DSA?
Regards, -Vladimir
|  |