Re: [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr

On Tue, Jun 15, 2021 at 04:50:53PM -0700, Tanner Love wrote:
[ ... ]

quoted

quoted

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 418b9b813d65..e1ac34548f9a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h

@@ -6017,6 +6017,8 @@ struct bpf_flow_keys {
      };
      __u32   flags;
      __be32  flow_label;
+     __bpf_md_ptr(const struct virtio_net_hdr *, vhdr);
+     __u8    vhdr_is_little_endian;

I am not familiar with virtio.  A question on the "vhdr_is_little_endian" field.
The commit message said
"to handle the case of a version 1+ header on a big endian machine".
iiuc, version 1+ is always in little endian?

That's right.

quoted

Does it mean most cases are in little endian?
and at least will eventually be moved to version 1+?

hmm..... so the common cases are little endian or
at least the remaining cases will eventually be moved to
version 1+ which is little endian?

quoted

I wonder if this field will eventually be useless (because of always
true) and can it be avoided from the uapi now.  The current uapi
fields (e.g. in bpf_sock) are always in one particular order.

If it is in big endian, can it be changed to little endian first
before calling the bpf prog?

In fact, v1 of this patch set did the conversion prior to passing
the fields to the bpf prog, which meant that the bpf prog did not
have to do anything about endianness. I changed that, though,
at the suggestion of Alexei; Alexei suggested that we pass a
pointer to struct virtio_net_hdr, rather than copying the individual
virtio_net_hdr fields. V1 did the endianness conversion as part
of that copying process. If we go back to doing it like that, then
we lose the advantage that Alexei's suggestion aimed to achieve
(i.e. avoiding the cost of copying the fields).

If the common case is little endian, then the vhdr pointer can still
be passed to bpf prog as-is directly for speed.  A deep copy and
conversion are only needed for the less common big endian case which
eventually will be moved to version 1+?