Messages in this thread | | | Date | Mon, 28 Nov 2022 10:12:21 +0100 | Subject | Re: [ovs-dev] [RFC net-next 1/6] openvswitch: exclude kernel flow key from upcalls | From | Adrian Moreno <> |
| |
On 11/25/22 16:51, Ilya Maximets wrote: > On 11/25/22 16:29, Adrian Moreno wrote: >> >> >> On 11/23/22 22:22, Ilya Maximets wrote: >>> On 11/22/22 15:03, Aaron Conole wrote: >>>> When processing upcall commands, two groups of data are available to >>>> userspace for processing: the actual packet data and the kernel >>>> sw flow key data. The inclusion of the flow key allows the userspace >>>> avoid running through the dissection again. >>>> >>>> However, the userspace can choose to ignore the flow key data, as is >>>> the case in some ovs-vswitchd upcall processing. For these messages, >>>> having the flow key data merely adds additional data to the upcall >>>> pipeline without any actual gain. Userspace simply throws the data >>>> away anyway. >>> >>> Hi, Aaron. While it's true that OVS in userpsace is re-parsing the >>> packet from scratch and using the newly parsed key for the OpenFlow >>> translation, the kernel-porvided key is still used in a few important >>> places. Mainly for the compatibility checking. The use is described >>> here in more details: >>> https://docs.kernel.org/networking/openvswitch.html#flow-key-compatibility >>> >>> We need to compare the key generated in userspace with the key >>> generated by the kernel to know if it's safe to install the new flow >>> to the kernel, i.e. if the kernel and OVS userpsace are parsing the >>> packet in the same way. >>> >> >> Hi Ilya, >> >> Do we need to do that for every packet? >> Could we send a bitmask of supported fields to userspace at feature >> negotiation and let OVS slowpath flows that it knows the kernel won't >> be able to handle properly? > > It's not that simple, because supported fields in a packet depend > on previous fields in that same packet. For example, parsing TCP > header is generally supported, but it won't be parsed for IPv6 > fragments (even the first one), number of vlan headers will affect > the parsing as we do not parse deeper than 2 vlan headers, etc. > So, I'm afraid we have to have a per-packet information, unless we > can somehow probe all the possible valid combinations of packet > headers. >
Surely. I understand that we'd need more than just a bit per field. Things like L4 on IPv6 frags would need another bit and the number of VLAN headers would need some more. But, are these a handful of exceptions or do we really need all the possible combinations of headers? If it's a matter of naming a handful of corner cases I think we could consider expressing them at initialization time and safe some buffer space plus computation time both in kernel and userspace.
-- Adrián Moreno
>> >> >>> On the other hand, OVS today doesn't check the data, it only checks >>> which fields are present. So, if we can generate and pass the bitmap >>> of fields present in the key or something similar without sending the >>> full key, that might still save some CPU cycles and memory in the >>> socket buffer while preserving the ability to check for forward and >>> backward compatibility. What do you think? >>> >>> >>> The rest of the patch set seems useful even without patch #1 though. >>> >>> Nit: This patch #1 should probably be merged with the patch #6 and be >>> at the end of a patch set, so the selftest and the main code are updated >>> at the same time. >>> >>> Best regards, Ilya Maximets. >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >> >> Thanks >
| |