Thread (11 messages) 11 messages, 2 authors, 2021-10-01

Re: [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation

flat view

From: Justin Iurman <hidden>
Date: 2021-10-01 11:38:49

quoted
quoted
quoted
 static const struct nla_policy ioam6_iptunnel_policy[IOAM6_IPTUNNEL_MAX + 1] = {
-	[IOAM6_IPTUNNEL_TRACE]	= NLA_POLICY_EXACT_LEN(sizeof(struct ioam6_trace_hdr)),
+	[IOAM6_IPTUNNEL_TRACE]	= NLA_POLICY_EXACT_LEN(sizeof(struct
ioam6_iptunnel_trace)),
you can't do that. Once a kernel is released with a given UAPI, it can
not be changed. You could go the other way and handle

struct ioam6_iptunnel_trace {
+	struct ioam6_trace_hdr trace;
+	__u8 mode;
+	struct in6_addr tundst;	/* unused for inline mode */
+};
Makes sense. But I'm not sure what you mean by "go the other way". Should I
handle ioam6_iptunnel_trace as well, in addition to ioam6_trace_hdr, so that
the uapi is backward compatible?
by "the other way" I meant let ioam6_trace_hdr be the top element in the
new ioam6_iptunnel_trace struct. If the IOAM6_IPTUNNEL_TRACE size ==
ioam6_trace_hdr then you know it is the legacy argument vs sizeof
ioam6_iptunnel_trace which is the new.
OK, I see. The problem is ioam6_trace_hdr must be the last entry because of its last field, which is "__u8 data[0]". But, anyway, I could still apply the same kind of logic with the size.
quoted
quoted
Also, no gaps in uapi. Make sure all holes are stated; an anonymous
entry is best.
Would something like this do the trick?

struct ioam6_iptunnel_trace {
	struct ioam6_trace_hdr trace;
	__u8 mode;
	union { /* anonymous field only used by both the encap and auto modes */
		struct in6_addr tundst;
	};
};
By anonymous filling of the holes I meant something like:

struct ioam6_iptunnel_trace {
struct ioam6_trace_hdr trace;
__u8 mode;
__u8 :8;
__u16 :16;

struct in6_addr tundst;
};

Use pahole to check that struct for proper alignment of the entries as
desired (4-byte or 8-byte aligned).
By reading your example, I'm not sure we're talking about the same thing. Actually, do you refer to the fact that the ioam6_trace_hdr field must be 8n-aligned? If so, I don't see any static way to do that (i.e., by adding anonymous fields as you did) since it depends on the size of the data field I mentioned above.  The size can either be 8n-aligned already, or 4n-aligned in which case we add a PadN (1 2 0 0) at the end of the data field.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help