[PATCH net-next 0/2] Support for the IOAM insertion frequency

[PATCH net-next 0/2] Support for the IOAM insertion frequency

[Justin Iurman]

The insertion frequency is represented as "k/n", meaning IOAM will be
added to {k} packets over {n} packets, with 0 < k <= n and 1 <= {k,n} <=
1000000. Therefore, it provides the following percentages of insertion
frequency: [0.0001% (min) ... 100% (max)].

Not only this solution allows an operator to apply dynamic frequencies
based on the current traffic load, but it also provides some
flexibility, i.e., by distinguishing similar cases (e.g., "1/2" and
"2/4").

"1/2" = Y N Y N Y N Y N ...
"2/4" = Y Y N N Y Y N N ...

Justin Iurman (2):
  uapi: ioam: Insertion frequency
  ipv6: ioam: Insertion frequency in lwtunnel output

 include/uapi/linux/ioam6_iptunnel.h |  9 +++++
 net/ipv6/ioam6_iptunnel.c           | 57 ++++++++++++++++++++++++++++-
 2 files changed, 64 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH net-next 1/2] uapi: ioam: Insertion frequency

[Justin Iurman]

Add the insertion frequency uapi for IOAM lwtunnels.

Signed-off-by: Justin Iurman <justin.iurman@uliege.be>
---
 include/uapi/linux/ioam6_iptunnel.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/uapi/linux/ioam6_iptunnel.h b/include/uapi/linux/ioam6_iptunnel.h
index 829ffdfcacca..462758cdba14 100644
--- a/include/uapi/linux/ioam6_iptunnel.h
+++ b/include/uapi/linux/ioam6_iptunnel.h
@@ -30,6 +30,15 @@ enum {
 enum {
 	IOAM6_IPTUNNEL_UNSPEC,
 
+	/* Insertion frequency:
+	 * "k over n" packets (0 < k <= n)
+	 * [0.0001% ... 100%]
+	 */
+#define IOAM6_IPTUNNEL_FREQ_MIN 1
+#define IOAM6_IPTUNNEL_FREQ_MAX 1000000
+	IOAM6_IPTUNNEL_FREQ_K,		/* s32 */
+	IOAM6_IPTUNNEL_FREQ_N,		/* s32 */
+
 	/* Encap mode */
 	IOAM6_IPTUNNEL_MODE,		/* u8 */
 
-- 
2.25.1

[PATCH net-next 2/2] ipv6: ioam: Insertion frequency in lwtunnel output

[Justin Iurman]

Add support for the IOAM insertion frequency inside its lwtunnel output
function. This patch introduces a new (atomic) counter for packets,
based on which the algorithm will decide if IOAM should be added or not.

Default frequency is "1/1" (i.e., applied to all packets) for backward
compatibility. The iproute2 patch is ready and will be submitted as soon
as this one is accepted.

Previous iproute2 command:
ip -6 ro ad fc00::1/128 encap ioam6 [ mode ... ] ...

New iproute2 command:
ip -6 ro ad fc00::1/128 encap ioam6 [ freq k/n ] [ mode ... ] ...

Signed-off-by: Justin Iurman <justin.iurman@uliege.be>
---
 net/ipv6/ioam6_iptunnel.c | 57 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c
index f90a87389fcc..3dfce3e4ae19 100644
--- a/net/ipv6/ioam6_iptunnel.c
+++ b/net/ipv6/ioam6_iptunnel.c
@@ -32,13 +32,25 @@ struct ioam6_lwt_encap {
 	struct ioam6_trace_hdr traceh;
 } __packed;
 
+struct ioam6_lwt_freq {
+	int k;
+	int n;
+};
+
 struct ioam6_lwt {
 	struct dst_cache cache;
+	struct ioam6_lwt_freq freq;
+	atomic_t pkt_cnt;
 	u8 mode;
 	struct in6_addr tundst;
 	struct ioam6_lwt_encap	tuninfo;
 };
 
+static struct netlink_range_validation_signed freq_range = {
+	.min = IOAM6_IPTUNNEL_FREQ_MIN,
+	.max = IOAM6_IPTUNNEL_FREQ_MAX,
+};
+
 static struct ioam6_lwt *ioam6_lwt_state(struct lwtunnel_state *lwt)
 {
 	return (struct ioam6_lwt *)lwt->data;
@@ -55,6 +67,8 @@ static struct ioam6_trace_hdr *ioam6_lwt_trace(struct lwtunnel_state *lwt)
 }
 
 static const struct nla_policy ioam6_iptunnel_policy[IOAM6_IPTUNNEL_MAX + 1] = {
+	[IOAM6_IPTUNNEL_FREQ_K] = NLA_POLICY_FULL_RANGE_SIGNED(NLA_S32, &freq_range),
+	[IOAM6_IPTUNNEL_FREQ_N] = NLA_POLICY_FULL_RANGE_SIGNED(NLA_S32, &freq_range),
 	[IOAM6_IPTUNNEL_MODE]	= NLA_POLICY_RANGE(NLA_U8,
 						   IOAM6_IPTUNNEL_MODE_MIN,
 						   IOAM6_IPTUNNEL_MODE_MAX),
@@ -96,6 +110,7 @@ static int ioam6_build_state(struct net *net, struct nlattr *nla,
 	struct lwtunnel_state *lwt;
 	struct ioam6_lwt *ilwt;
 	int len_aligned, err;
+	int freq_k, freq_n;
 	u8 mode;
 
 	if (family != AF_INET6)
@@ -106,6 +121,23 @@ static int ioam6_build_state(struct net *net, struct nlattr *nla,
 	if (err < 0)
 		return err;
 
+	if ((!tb[IOAM6_IPTUNNEL_FREQ_K] && tb[IOAM6_IPTUNNEL_FREQ_N]) ||
+	    (tb[IOAM6_IPTUNNEL_FREQ_K] && !tb[IOAM6_IPTUNNEL_FREQ_N])) {
+		NL_SET_ERR_MSG(extack, "freq: missing parameter");
+		return -EINVAL;
+	} else if (!tb[IOAM6_IPTUNNEL_FREQ_K] && !tb[IOAM6_IPTUNNEL_FREQ_N]) {
+		freq_k = IOAM6_IPTUNNEL_FREQ_MIN;
+		freq_n = IOAM6_IPTUNNEL_FREQ_MIN;
+	} else {
+		freq_k = nla_get_s32(tb[IOAM6_IPTUNNEL_FREQ_K]);
+		freq_n = nla_get_s32(tb[IOAM6_IPTUNNEL_FREQ_N]);
+
+		if (freq_k > freq_n) {
+			NL_SET_ERR_MSG(extack, "freq: k > n is forbidden");
+			return -EINVAL;
+		}
+	}
+
 	if (!tb[IOAM6_IPTUNNEL_MODE])
 		mode = IOAM6_IPTUNNEL_MODE_INLINE;
 	else
@@ -140,6 +172,10 @@ static int ioam6_build_state(struct net *net, struct nlattr *nla,
 		return err;
 	}
 
+	atomic_set(&ilwt->pkt_cnt, 0);
+	ilwt->freq.k = freq_k;
+	ilwt->freq.n = freq_n;
+
 	ilwt->mode = mode;
 	if (tb[IOAM6_IPTUNNEL_DST])
 		ilwt->tundst = nla_get_in6_addr(tb[IOAM6_IPTUNNEL_DST]);
@@ -268,6 +304,11 @@ static int ioam6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
 		goto drop;
 
 	ilwt = ioam6_lwt_state(dst->lwtstate);
+
+	/* Check for insertion frequency (i.e., "k over n" insertions) */
+	if (atomic_fetch_inc(&ilwt->pkt_cnt) % ilwt->freq.n >= ilwt->freq.k)
+		goto out;
+
 	orig_daddr = ipv6_hdr(skb)->daddr;
 
 	switch (ilwt->mode) {
@@ -358,6 +399,14 @@ static int ioam6_fill_encap_info(struct sk_buff *skb,
 	struct ioam6_lwt *ilwt = ioam6_lwt_state(lwtstate);
 	int err;
 
+	err = nla_put_s32(skb, IOAM6_IPTUNNEL_FREQ_K, ilwt->freq.k);
+	if (err)
+		goto ret;
+
+	err = nla_put_s32(skb, IOAM6_IPTUNNEL_FREQ_N, ilwt->freq.n);
+	if (err)
+		goto ret;
+
 	err = nla_put_u8(skb, IOAM6_IPTUNNEL_MODE, ilwt->mode);
 	if (err)
 		goto ret;
@@ -379,7 +428,9 @@ static int ioam6_encap_nlsize(struct lwtunnel_state *lwtstate)
 	struct ioam6_lwt *ilwt = ioam6_lwt_state(lwtstate);
 	int nlsize;
 
-	nlsize = nla_total_size(sizeof(ilwt->mode)) +
+	nlsize = nla_total_size(sizeof(ilwt->freq.k)) +
+		  nla_total_size(sizeof(ilwt->freq.n)) +
+		  nla_total_size(sizeof(ilwt->mode)) +
 		  nla_total_size(sizeof(ilwt->tuninfo.traceh));
 
 	if (ilwt->mode != IOAM6_IPTUNNEL_MODE_INLINE)
@@ -395,7 +446,9 @@ static int ioam6_encap_cmp(struct lwtunnel_state *a, struct lwtunnel_state *b)
 	struct ioam6_lwt *ilwt_a = ioam6_lwt_state(a);
 	struct ioam6_lwt *ilwt_b = ioam6_lwt_state(b);
 
-	return (ilwt_a->mode != ilwt_b->mode ||
+	return (ilwt_a->freq.k != ilwt_b->freq.k ||
+		ilwt_a->freq.n != ilwt_b->freq.n ||
+		ilwt_a->mode != ilwt_b->mode ||
 		(ilwt_a->mode != IOAM6_IPTUNNEL_MODE_INLINE &&
 		 !ipv6_addr_equal(&ilwt_a->tundst, &ilwt_b->tundst)) ||
 		trace_a->namespace_id != trace_b->namespace_id);
-- 
2.25.1

Re: [PATCH net-next 1/2] uapi: ioam: Insertion frequency

[Jakub Kicinski]

On Wed, 26 Jan 2022 19:46:27 +0100 Justin Iurman wrote:
> Add the insertion frequency uapi for IOAM lwtunnels.
> 
> Signed-off-by: Justin Iurman <justin.iurman@uliege.be>
> ---
>  include/uapi/linux/ioam6_iptunnel.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/uapi/linux/ioam6_iptunnel.h b/include/uapi/linux/ioam6_iptunnel.h
> index 829ffdfcacca..462758cdba14 100644
> --- a/include/uapi/linux/ioam6_iptunnel.h
> +++ b/include/uapi/linux/ioam6_iptunnel.h
> @@ -30,6 +30,15 @@ enum {
>  enum {
>  	IOAM6_IPTUNNEL_UNSPEC,
>  
> +	/* Insertion frequency:
> +	 * "k over n" packets (0 < k <= n)
> +	 * [0.0001% ... 100%]
> +	 */
> +#define IOAM6_IPTUNNEL_FREQ_MIN 1
> +#define IOAM6_IPTUNNEL_FREQ_MAX 1000000

If min is 1 why not make the value unsigned?

> +	IOAM6_IPTUNNEL_FREQ_K,		/* s32 */
> +	IOAM6_IPTUNNEL_FREQ_N,		/* s32 */

You can't insert into the middle of a uAPI enum. Binary compatibility.

>  	/* Encap mode */
>  	IOAM6_IPTUNNEL_MODE,		/* u8 */
>  

Re: [PATCH net-next 1/2] uapi: ioam: Insertion frequency

[Justin Iurman]

On Jan 29, 2022, at 2:31 AM, Jakub Kicinski kuba@kernel.org wrote:
> On Wed, 26 Jan 2022 19:46:27 +0100 Justin Iurman wrote:
>> Add the insertion frequency uapi for IOAM lwtunnels.
>> 
>> Signed-off-by: Justin Iurman <justin.iurman@uliege.be>
>> ---
>>  include/uapi/linux/ioam6_iptunnel.h | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>> 
>> diff --git a/include/uapi/linux/ioam6_iptunnel.h
>> b/include/uapi/linux/ioam6_iptunnel.h
>> index 829ffdfcacca..462758cdba14 100644
>> --- a/include/uapi/linux/ioam6_iptunnel.h
>> +++ b/include/uapi/linux/ioam6_iptunnel.h
>> @@ -30,6 +30,15 @@ enum {
>>  enum {
>>  	IOAM6_IPTUNNEL_UNSPEC,
>>  
>> +	/* Insertion frequency:
>> +	 * "k over n" packets (0 < k <= n)
>> +	 * [0.0001% ... 100%]
>> +	 */
>> +#define IOAM6_IPTUNNEL_FREQ_MIN 1
>> +#define IOAM6_IPTUNNEL_FREQ_MAX 1000000
> 
> If min is 1 why not make the value unsigned?

The atomic_t type is just a wrapper for a signed int, so I didn't want
to have to convert from signed to unsigned. I agree it'd sound better to
have unsigned here, though.

>> +	IOAM6_IPTUNNEL_FREQ_K,		/* s32 */
>> +	IOAM6_IPTUNNEL_FREQ_N,		/* s32 */
> 
> You can't insert into the middle of a uAPI enum. Binary compatibility.

Is it really the middle? I recall adding the "mode" at the top (still
below the "_UNSPEC"), which I thought was correct at that time (and had
no objection). That's why I did the same here. Should I move it to the
end, then?

>>  	/* Encap mode */
>>  	IOAM6_IPTUNNEL_MODE,		/* u8 */

Re: [PATCH net-next 1/2] uapi: ioam: Insertion frequency

[Justin Iurman]

>>> diff --git a/include/uapi/linux/ioam6_iptunnel.h
>>> b/include/uapi/linux/ioam6_iptunnel.h
>>> index 829ffdfcacca..462758cdba14 100644
>>> --- a/include/uapi/linux/ioam6_iptunnel.h
>>> +++ b/include/uapi/linux/ioam6_iptunnel.h
>>> @@ -30,6 +30,15 @@ enum {
>>>  enum {
>>>  	IOAM6_IPTUNNEL_UNSPEC,
>>>  
>>> +	/* Insertion frequency:
>>> +	 * "k over n" packets (0 < k <= n)
>>> +	 * [0.0001% ... 100%]
>>> +	 */
>>> +#define IOAM6_IPTUNNEL_FREQ_MIN 1
>>> +#define IOAM6_IPTUNNEL_FREQ_MAX 1000000
>> 
>> If min is 1 why not make the value unsigned?
> 
> The atomic_t type is just a wrapper for a signed int, so I didn't want
> to have to convert from signed to unsigned. I agree it'd sound better to
> have unsigned here, though.

Sorry, I figured out a cast is fine thanks to [1] which says:

"[...] the kernel uses -fno-strict-overflow
(which implies -fwrapv) and defines signed overflow to behave like
2s-complement.
Therefore, an explicitly unsigned variant of the atomic ops is strictly
unnecessary and we can simply cast, there is no UB. [...]"

  [1] https://www.kernel.org/doc/Documentation/atomic_t.txt

Re: [PATCH net-next 1/2] uapi: ioam: Insertion frequency

[Jakub Kicinski]

On Sat, 29 Jan 2022 12:24:47 +0100 (CET) Justin Iurman wrote:
> >> +	IOAM6_IPTUNNEL_FREQ_K,		/* s32 */
> >> +	IOAM6_IPTUNNEL_FREQ_N,		/* s32 */  
> > 
> > You can't insert into the middle of a uAPI enum. Binary compatibility.  
> 
> Is it really the middle? I recall adding the "mode" at the top (still
> below the "_UNSPEC"), which I thought was correct at that time (and had
> no objection).

Maybe because both changes were made in the same kernel release?
Not sure.

> That's why I did the same here. Should I move it to the end, then?

You have to move it. I don't see how this patch as is wouldn't change
the value of IOAM6_IPTUNNEL_MODE.

Re: [PATCH net-next 1/2] uapi: ioam: Insertion frequency

[Justin Iurman]

On Jan 31, 2022, at 7:54 PM, Jakub Kicinski kuba@kernel.org wrote:
>> >> +	IOAM6_IPTUNNEL_FREQ_K,		/* s32 */
>> >> +	IOAM6_IPTUNNEL_FREQ_N,		/* s32 */
>> > 
>> > You can't insert into the middle of a uAPI enum. Binary compatibility.
>> 
>> Is it really the middle? I recall adding the "mode" at the top (still
>> below the "_UNSPEC"), which I thought was correct at that time (and had
>> no objection).
> 
> Maybe because both changes were made in the same kernel release?
> Not sure.

I just checked. They were both in two different releases, i.e., 5.15 and
5.16. That's weird. It means that the value of IOAM6_IPTUNNEL_TRACE has
changed between 5.15 and 5.16, where it shouldn't have, right? Anyway...

>> That's why I did the same here. Should I move it to the end, then?
> 
> You have to move it. I don't see how this patch as is wouldn't change
> the value of IOAM6_IPTUNNEL_MODE.

Indeed. So moving it below IOAM6_IPTUNNEL_TRACE should be fine I guess.