Re: [PATCH net-next v4] openvswitch: enable NSH support

On Tue, Aug 22, 2017 at 08:32:49AM +0000, Jan Scheurich wrote:

quoted

quoted

Or why else does OVS user space code take so great pain to model
possible misalignment and provide/use safe access functions?

I don't know how the ovs user space deals with packet allocation. In
the kernel, the network header is aligned in a way that it allows
efficient 32bit access.

It seems that OVS has not had the same approach as Linux. There is no
config parameter covering the alignment characteristics of the machine
architecture. For packets buffers received from outside sources
(e.g. DPDK interfaces) they make no assumptions on alignment and play
safe. For packets allocated inside OVS, the Ethernet packet is
typically stored so that the L3 header is 32-bit aligned, so that the
misalignment precautions would be unnecessary. But I didn't check all
code paths.

We solved the alignment problem in OVS userspace a different way, by
defining our versions of the network protocol headers so that they only
need 16-bit alignment.  In turn, we did that by defining a
ovs_16aligned_be32 type as a pair of be16s and ovs_16aligned_be64 as
four be16s, and using helper functions for reads and writes.  This made
it harder to screw up alignment in a subtle way and only find out long
after release when someone tested a corner case on a RISC architecture.
It probably has a performance cost on those RISC architectures, for the
cases where the access really is aligned, but it's more obviously
correct and I highly value that for OVS userspace.

As far as I can tell it's not actually possible, in the general case, to
add padding such that all parts of a packet are aligned.  VXLAN is the
case that comes to mind.  With VXLAN, as far as I can tell, the 14-byte
inner Ethernet header mean that you can align either the outer IPv4
header or the inner IPv4 header, but not both.  That means that no
matter how careful OVS is about aligning packets, it would still have to
deal with unaligned accesses in some cases.

I see that the VXLAN issue has come up in Linux before:
https://www.ietf.org/mail-archive/web/nvo3/current/msg05743.html

From: Ben Pfaff

Sent: 22 August 2017 18:35

...

We solved the alignment problem in OVS userspace a different way, by
defining our versions of the network protocol headers so that they only
need 16-bit alignment.  In turn, we did that by defining a
ovs_16aligned_be32 type as a pair of be16s and ovs_16aligned_be64 as
four be16s, and using helper functions for reads and writes.

...

If you add __attribute__((aligned(2))) to the 32bit and 64bit structure
members then gcc will do the loads and shifts for you.

	David