* [PATCH net-next 0/2] Support for the ip6ip6 encapsulation of IOAM
@ 2021-09-28 19:03 Justin Iurman
2021-09-28 19:03 ` [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation Justin Iurman
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Justin Iurman @ 2021-09-28 19:03 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, yoshfuji, dsahern, justin.iurman
In the current implementation, IOAM can only be inserted directly (i.e., only
inside packets generated locally) by default, to be compliant with RFC8200.
This patch adds support for in-transit packets and provides the ip6ip6
encapsulation of IOAM. Therefore, three explicit encap modes are defined:
- inline: directly inserts IOAM inside packets.
- encap: ip6ip6 encapsulation of IOAM inside packets.
- auto: either inline mode for packets generated locally or encap mode for
in-transit packets.
With current iproute2 implementation, it is configured this way:
$ ip -6 r [...] encap ioam6 trace prealloc type 0x800000 ns 1 size 12 [...]
Now, an encap mode must be specified:
(inline mode)
$ [...] encap ioam6 mode inline trace prealloc [...]
(encap mode)
$ [...] encap ioam6 mode encap tundst fc00::2 trace prealloc [...]
(auto mode)
$ [...] encap ioam6 mode auto tundst fc00::2 trace prealloc [...]
A tunnel destination address must be configured when using the encap mode or the
auto mode.
Justin Iurman (2):
ipv6: ioam: Add support for the ip6ip6 encapsulation
selftests: net: Test for the IOAM encapsulation with IPv6
include/net/ioam6.h | 3 +-
include/uapi/linux/ioam6_iptunnel.h | 19 +-
net/ipv6/Kconfig | 6 +-
net/ipv6/exthdrs.c | 2 +-
net/ipv6/ioam6.c | 11 +-
net/ipv6/ioam6_iptunnel.c | 299 ++++++++++++++++++++-------
tools/testing/selftests/net/ioam6.sh | 209 ++++++++++++++-----
7 files changed, 409 insertions(+), 140 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation
2021-09-28 19:03 [PATCH net-next 0/2] Support for the ip6ip6 encapsulation of IOAM Justin Iurman
@ 2021-09-28 19:03 ` Justin Iurman
2021-09-30 3:26 ` David Ahern
2021-09-28 19:03 ` [PATCH net-next 2/2] selftests: net: Test for the IOAM encapsulation with IPv6 Justin Iurman
2021-09-30 3:20 ` [PATCH net-next 0/2] Support for the ip6ip6 encapsulation of IOAM David Ahern
2 siblings, 1 reply; 11+ messages in thread
From: Justin Iurman @ 2021-09-28 19:03 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, yoshfuji, dsahern, justin.iurman
This patch adds support for the ip6ip6 encapsulation by providing three encap
modes: inline, encap and auto.
Signed-off-by: Justin Iurman <justin.iurman@uliege.be>
---
include/net/ioam6.h | 3 +-
include/uapi/linux/ioam6_iptunnel.h | 19 +-
net/ipv6/Kconfig | 6 +-
net/ipv6/exthdrs.c | 2 +-
net/ipv6/ioam6.c | 11 +-
net/ipv6/ioam6_iptunnel.c | 299 ++++++++++++++++++++--------
6 files changed, 250 insertions(+), 90 deletions(-)
diff --git a/include/net/ioam6.h b/include/net/ioam6.h
index 3c2993bc48c8..3f45ba37a2c6 100644
--- a/include/net/ioam6.h
+++ b/include/net/ioam6.h
@@ -56,7 +56,8 @@ static inline struct ioam6_pernet_data *ioam6_pernet(struct net *net)
struct ioam6_namespace *ioam6_namespace(struct net *net, __be16 id);
void ioam6_fill_trace_data(struct sk_buff *skb,
struct ioam6_namespace *ns,
- struct ioam6_trace_hdr *trace);
+ struct ioam6_trace_hdr *trace,
+ bool is_input);
int ioam6_init(void);
void ioam6_exit(void);
diff --git a/include/uapi/linux/ioam6_iptunnel.h b/include/uapi/linux/ioam6_iptunnel.h
index bae14636a8c8..4fb7e78018b5 100644
--- a/include/uapi/linux/ioam6_iptunnel.h
+++ b/include/uapi/linux/ioam6_iptunnel.h
@@ -9,9 +9,26 @@
#ifndef _UAPI_LINUX_IOAM6_IPTUNNEL_H
#define _UAPI_LINUX_IOAM6_IPTUNNEL_H
+#include <linux/in6.h>
+#include <linux/ioam6.h>
+#include <linux/types.h>
+
+enum {
+ IOAM6_IPTUNNEL_MODE_UNSPEC,
+ IOAM6_IPTUNNEL_MODE_INLINE, /* direct insertion only */
+ IOAM6_IPTUNNEL_MODE_ENCAP, /* encap (ip6ip6) only */
+ IOAM6_IPTUNNEL_MODE_AUTO, /* inline or encap based on situation */
+};
+
+struct ioam6_iptunnel_trace {
+ __u8 mode;
+ struct in6_addr tundst; /* unused for inline mode */
+ struct ioam6_trace_hdr trace;
+};
+
enum {
IOAM6_IPTUNNEL_UNSPEC,
- IOAM6_IPTUNNEL_TRACE, /* struct ioam6_trace_hdr */
+ IOAM6_IPTUNNEL_TRACE,
__IOAM6_IPTUNNEL_MAX,
};
diff --git a/net/ipv6/Kconfig b/net/ipv6/Kconfig
index e504204bca92..bf2e5e5fe142 100644
--- a/net/ipv6/Kconfig
+++ b/net/ipv6/Kconfig
@@ -332,10 +332,10 @@ config IPV6_IOAM6_LWTUNNEL
bool "IPv6: IOAM Pre-allocated Trace insertion support"
depends on IPV6
select LWTUNNEL
+ select DST_CACHE
help
- Support for the inline insertion of IOAM Pre-allocated
- Trace Header (only on locally generated packets), using
- the lightweight tunnels mechanism.
+ Support for the insertion of IOAM Pre-allocated Trace
+ Header using the lightweight tunnels mechanism.
If unsure, say N.
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 3a871a09f962..38ece3b7b839 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -979,7 +979,7 @@ static bool ipv6_hop_ioam(struct sk_buff *skb, int optoff)
if (!skb_valid_dst(skb))
ip6_route_input(skb);
- ioam6_fill_trace_data(skb, ns, trace);
+ ioam6_fill_trace_data(skb, ns, trace, true);
break;
default:
break;
diff --git a/net/ipv6/ioam6.c b/net/ipv6/ioam6.c
index 5e8961004832..4e5583dbadac 100644
--- a/net/ipv6/ioam6.c
+++ b/net/ipv6/ioam6.c
@@ -631,7 +631,7 @@ static void __ioam6_fill_trace_data(struct sk_buff *skb,
struct ioam6_namespace *ns,
struct ioam6_trace_hdr *trace,
struct ioam6_schema *sc,
- u8 sclen)
+ u8 sclen, bool is_input)
{
struct __kernel_sock_timeval ts;
u64 raw64;
@@ -645,7 +645,7 @@ static void __ioam6_fill_trace_data(struct sk_buff *skb,
/* hop_lim and node_id */
if (trace->type.bit0) {
byte = ipv6_hdr(skb)->hop_limit;
- if (skb->dev)
+ if (is_input)
byte--;
raw32 = dev_net(skb_dst(skb)->dev)->ipv6.sysctl.ioam6_id;
@@ -730,7 +730,7 @@ static void __ioam6_fill_trace_data(struct sk_buff *skb,
/* hop_lim and node_id (wide) */
if (trace->type.bit8) {
byte = ipv6_hdr(skb)->hop_limit;
- if (skb->dev)
+ if (is_input)
byte--;
raw64 = dev_net(skb_dst(skb)->dev)->ipv6.sysctl.ioam6_id_wide;
@@ -786,7 +786,8 @@ static void __ioam6_fill_trace_data(struct sk_buff *skb,
/* called with rcu_read_lock() */
void ioam6_fill_trace_data(struct sk_buff *skb,
struct ioam6_namespace *ns,
- struct ioam6_trace_hdr *trace)
+ struct ioam6_trace_hdr *trace,
+ bool is_input)
{
struct ioam6_schema *sc;
u8 sclen = 0;
@@ -822,7 +823,7 @@ void ioam6_fill_trace_data(struct sk_buff *skb,
return;
}
- __ioam6_fill_trace_data(skb, ns, trace, sc, sclen);
+ __ioam6_fill_trace_data(skb, ns, trace, sc, sclen, is_input);
trace->remlen -= trace->nodelen + sclen;
}
diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c
index f9ee04541c17..923a5aedad9e 100644
--- a/net/ipv6/ioam6_iptunnel.c
+++ b/net/ipv6/ioam6_iptunnel.c
@@ -17,18 +17,25 @@
#include <net/sock.h>
#include <net/lwtunnel.h>
#include <net/ioam6.h>
+#include <net/ipv6.h>
+#include <net/dst_cache.h>
+#include <net/ip6_route.h>
+#include <net/addrconf.h>
#define IOAM6_MASK_SHORT_FIELDS 0xff100000
#define IOAM6_MASK_WIDE_FIELDS 0xe00000
struct ioam6_lwt_encap {
- struct ipv6_hopopt_hdr eh;
- u8 pad[2]; /* 2-octet padding for 4n-alignment */
- struct ioam6_hdr ioamh;
- struct ioam6_trace_hdr traceh;
+ struct ipv6_hopopt_hdr eh;
+ u8 pad[2]; /* 2-octet padding for 4n-alignment */
+ struct ioam6_hdr ioamh;
+ struct ioam6_trace_hdr traceh;
} __packed;
struct ioam6_lwt {
+ u8 mode;
+ struct in6_addr tundst;
+ struct dst_cache cache;
struct ioam6_lwt_encap tuninfo;
};
@@ -42,34 +49,15 @@ static struct ioam6_lwt_encap *ioam6_lwt_info(struct lwtunnel_state *lwt)
return &ioam6_lwt_state(lwt)->tuninfo;
}
-static struct ioam6_trace_hdr *ioam6_trace(struct lwtunnel_state *lwt)
+static struct ioam6_trace_hdr *ioam6_lwt_trace(struct lwtunnel_state *lwt)
{
return &(ioam6_lwt_state(lwt)->tuninfo.traceh);
}
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)),
};
-static int nla_put_ioam6_trace(struct sk_buff *skb, int attrtype,
- struct ioam6_trace_hdr *trace)
-{
- struct ioam6_trace_hdr *data;
- struct nlattr *nla;
- int len;
-
- len = sizeof(*trace);
-
- nla = nla_reserve(skb, attrtype, len);
- if (!nla)
- return -EMSGSIZE;
-
- data = nla_data(nla);
- memcpy(data, trace, len);
-
- return 0;
-}
-
static bool ioam6_validate_trace_hdr(struct ioam6_trace_hdr *trace)
{
u32 fields;
@@ -95,11 +83,12 @@ static int ioam6_build_state(struct net *net, struct nlattr *nla,
struct netlink_ext_ack *extack)
{
struct nlattr *tb[IOAM6_IPTUNNEL_MAX + 1];
- struct ioam6_lwt_encap *tuninfo;
+ struct ioam6_iptunnel_trace *data;
struct ioam6_trace_hdr *trace;
+ struct ioam6_lwt_encap *info;
struct lwtunnel_state *s;
- int len_aligned;
- int len, err;
+ struct ioam6_lwt *lwt;
+ int len, aligned, err;
if (family != AF_INET6)
return -EINVAL;
@@ -114,36 +103,59 @@ static int ioam6_build_state(struct net *net, struct nlattr *nla,
return -EINVAL;
}
- trace = nla_data(tb[IOAM6_IPTUNNEL_TRACE]);
- if (!ioam6_validate_trace_hdr(trace)) {
+ data = nla_data(tb[IOAM6_IPTUNNEL_TRACE]);
+ if (!ioam6_validate_trace_hdr(&data->trace)) {
NL_SET_ERR_MSG_ATTR(extack, tb[IOAM6_IPTUNNEL_TRACE],
"invalid trace validation");
return -EINVAL;
}
- len = sizeof(*tuninfo) + trace->remlen * 4;
- len_aligned = ALIGN(len, 8);
+ switch (data->mode) {
+ case IOAM6_IPTUNNEL_MODE_INLINE:
+ case IOAM6_IPTUNNEL_MODE_ENCAP:
+ case IOAM6_IPTUNNEL_MODE_AUTO:
+ break;
+ default:
+ NL_SET_ERR_MSG_ATTR(extack, tb[IOAM6_IPTUNNEL_TRACE],
+ "invalid mode");
+ return -EINVAL;
+ }
+
+ len = sizeof(*info) + data->trace.remlen * 4;
+ aligned = ALIGN(len, 8);
- s = lwtunnel_state_alloc(len_aligned);
+ s = lwtunnel_state_alloc(aligned + sizeof(*lwt) - sizeof(*info));
if (!s)
return -ENOMEM;
- tuninfo = ioam6_lwt_info(s);
- tuninfo->eh.hdrlen = (len_aligned >> 3) - 1;
- tuninfo->pad[0] = IPV6_TLV_PADN;
- tuninfo->ioamh.type = IOAM6_TYPE_PREALLOC;
- tuninfo->ioamh.opt_type = IPV6_TLV_IOAM;
- tuninfo->ioamh.opt_len = sizeof(tuninfo->ioamh) - 2 + sizeof(*trace)
- + trace->remlen * 4;
+ lwt = ioam6_lwt_state(s);
+ lwt->mode = data->mode;
+ if (lwt->mode != IOAM6_IPTUNNEL_MODE_INLINE)
+ memcpy(&lwt->tundst, &data->tundst, sizeof(lwt->tundst));
+
+ err = dst_cache_init(&lwt->cache, GFP_ATOMIC);
+ if (err) {
+ kfree(s);
+ return err;
+ }
+
+ trace = ioam6_lwt_trace(s);
+ memcpy(trace, &data->trace, sizeof(*trace));
- memcpy(&tuninfo->traceh, trace, sizeof(*trace));
+ info = ioam6_lwt_info(s);
+ info->eh.hdrlen = (aligned >> 3) - 1;
+ info->pad[0] = IPV6_TLV_PADN;
+ info->ioamh.type = IOAM6_TYPE_PREALLOC;
+ info->ioamh.opt_type = IPV6_TLV_IOAM;
+ info->ioamh.opt_len = sizeof(info->ioamh) - 2;
+ info->ioamh.opt_len += sizeof(*trace) + trace->remlen * 4;
- len = len_aligned - len;
+ len = aligned - len;
if (len == 1) {
- tuninfo->traceh.data[trace->remlen * 4] = IPV6_TLV_PAD1;
+ trace->data[trace->remlen * 4] = IPV6_TLV_PAD1;
} else if (len > 0) {
- tuninfo->traceh.data[trace->remlen * 4] = IPV6_TLV_PADN;
- tuninfo->traceh.data[trace->remlen * 4 + 1] = len - 2;
+ trace->data[trace->remlen * 4] = IPV6_TLV_PADN;
+ trace->data[trace->remlen * 4 + 1] = len - 2;
}
s->type = LWTUNNEL_ENCAP_IOAM6;
@@ -154,11 +166,26 @@ static int ioam6_build_state(struct net *net, struct nlattr *nla,
return 0;
}
-static int ioam6_do_inline(struct sk_buff *skb, struct ioam6_lwt_encap *tuninfo)
+static int ioam6_do_fill(struct net *net, struct sk_buff *skb)
{
struct ioam6_trace_hdr *trace;
- struct ipv6hdr *oldhdr, *hdr;
struct ioam6_namespace *ns;
+
+ trace = (struct ioam6_trace_hdr *)(skb_transport_header(skb)
+ + sizeof(struct ipv6_hopopt_hdr) + 2
+ + sizeof(struct ioam6_hdr));
+
+ ns = ioam6_namespace(net, trace->namespace_id);
+ if (ns)
+ ioam6_fill_trace_data(skb, ns, trace, false);
+
+ return 0;
+}
+
+static int ioam6_do_inline(struct net *net, struct sk_buff *skb,
+ struct ioam6_lwt_encap *tuninfo)
+{
+ struct ipv6hdr *oldhdr, *hdr;
int hdrlen, err;
hdrlen = (tuninfo->eh.hdrlen + 1) << 3;
@@ -187,80 +214,194 @@ static int ioam6_do_inline(struct sk_buff *skb, struct ioam6_lwt_encap *tuninfo)
hdr->nexthdr = NEXTHDR_HOP;
hdr->payload_len = cpu_to_be16(skb->len - sizeof(*hdr));
- trace = (struct ioam6_trace_hdr *)(skb_transport_header(skb)
- + sizeof(struct ipv6_hopopt_hdr) + 2
- + sizeof(struct ioam6_hdr));
+ return ioam6_do_fill(net, skb);
+}
- ns = ioam6_namespace(dev_net(skb_dst(skb)->dev), trace->namespace_id);
- if (ns)
- ioam6_fill_trace_data(skb, ns, trace);
+static int ioam6_do_encap(struct net *net, struct sk_buff *skb,
+ struct ioam6_lwt_encap *tuninfo,
+ struct in6_addr *tundst)
+{
+ struct dst_entry *dst = skb_dst(skb);
+ struct ipv6hdr *hdr, *inner_hdr;
+ int hdrlen, len, err;
- return 0;
+ hdrlen = (tuninfo->eh.hdrlen + 1) << 3;
+ len = sizeof(*hdr) + hdrlen;
+
+ err = skb_cow_head(skb, len + skb->mac_len);
+ if (unlikely(err))
+ return err;
+
+ inner_hdr = ipv6_hdr(skb);
+
+ skb_push(skb, len);
+ skb_reset_network_header(skb);
+ skb_mac_header_rebuild(skb);
+ skb_set_transport_header(skb, sizeof(*hdr));
+
+ tuninfo->eh.nexthdr = NEXTHDR_IPV6;
+ memcpy(skb_transport_header(skb), (u8 *)tuninfo, hdrlen);
+
+ hdr = ipv6_hdr(skb);
+ memcpy(hdr, inner_hdr, sizeof(*hdr));
+
+ hdr->nexthdr = NEXTHDR_HOP;
+ hdr->payload_len = cpu_to_be16(skb->len - sizeof(*hdr));
+ hdr->daddr = *tundst;
+ ipv6_dev_get_saddr(net, dst->dev, &hdr->daddr,
+ IPV6_PREFER_SRC_PUBLIC, &hdr->saddr);
+
+ skb_postpush_rcsum(skb, hdr, len);
+
+ return ioam6_do_fill(net, skb);
}
static int ioam6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
{
- struct lwtunnel_state *lwt = skb_dst(skb)->lwtstate;
+ struct dst_entry *dst = skb_dst(skb);
+ struct in6_addr daddr_prev;
+ struct ioam6_lwt *lwt;
int err = -EINVAL;
+ lwt = ioam6_lwt_state(dst->lwtstate);
+
if (skb->protocol != htons(ETH_P_IPV6))
goto drop;
- /* Only for packets we send and
- * that do not contain a Hop-by-Hop yet
- */
- if (skb->dev || ipv6_hdr(skb)->nexthdr == NEXTHDR_HOP)
- goto out;
-
- err = ioam6_do_inline(skb, ioam6_lwt_info(lwt));
- if (unlikely(err))
+ daddr_prev = ipv6_hdr(skb)->daddr;
+
+ switch (lwt->mode) {
+ case IOAM6_IPTUNNEL_MODE_INLINE:
+do_inline:
+ /* Direct insertion - if there is no Hop-by-Hop yet */
+ if (ipv6_hdr(skb)->nexthdr == NEXTHDR_HOP)
+ goto out;
+
+ err = ioam6_do_inline(net, skb, &lwt->tuninfo);
+ if (unlikely(err))
+ goto drop;
+
+ break;
+ case IOAM6_IPTUNNEL_MODE_ENCAP:
+do_encap:
+ /* Encapsulation (ip6ip6) */
+ err = ioam6_do_encap(net, skb, &lwt->tuninfo, &lwt->tundst);
+ if (unlikely(err))
+ goto drop;
+
+ break;
+ case IOAM6_IPTUNNEL_MODE_AUTO:
+ /* Automatic (RFC8200 compliant):
+ * - local packet -> INLINE mode
+ * - forwarded packet -> ENCAP mode
+ */
+ if (!skb->dev)
+ goto do_inline;
+
+ goto do_encap;
+ default:
goto drop;
+ }
- err = skb_cow_head(skb, LL_RESERVED_SPACE(skb_dst(skb)->dev));
+ err = skb_cow_head(skb, LL_RESERVED_SPACE(dst->dev));
if (unlikely(err))
goto drop;
+ if (!ipv6_addr_equal(&daddr_prev, &ipv6_hdr(skb)->daddr)) {
+ preempt_disable();
+ dst = dst_cache_get(&lwt->cache);
+ preempt_enable();
+
+ if (unlikely(!dst)) {
+ struct ipv6hdr *hdr = ipv6_hdr(skb);
+ struct flowi6 fl6;
+
+ memset(&fl6, 0, sizeof(fl6));
+ fl6.daddr = hdr->daddr;
+ fl6.saddr = hdr->saddr;
+ fl6.flowlabel = ip6_flowinfo(hdr);
+ fl6.flowi6_mark = skb->mark;
+ fl6.flowi6_proto = hdr->nexthdr;
+
+ dst = ip6_route_output(net, NULL, &fl6);
+ if (dst->error) {
+ err = dst->error;
+ dst_release(dst);
+ goto drop;
+ }
+
+ preempt_disable();
+ dst_cache_set_ip6(&lwt->cache, dst, &fl6.saddr);
+ preempt_enable();
+ }
+
+ skb_dst_drop(skb);
+ skb_dst_set(skb, dst);
+
+ return dst_output(net, sk, skb);
+ }
out:
- return lwt->orig_output(net, sk, skb);
-
+ return dst->lwtstate->orig_output(net, sk, skb);
drop:
kfree_skb(skb);
return err;
}
+static void ioam6_destroy_state(struct lwtunnel_state *lwt)
+{
+ dst_cache_destroy(&ioam6_lwt_state(lwt)->cache);
+}
+
static int ioam6_fill_encap_info(struct sk_buff *skb,
struct lwtunnel_state *lwtstate)
{
- struct ioam6_trace_hdr *trace = ioam6_trace(lwtstate);
+ struct ioam6_iptunnel_trace *info;
+ struct ioam6_trace_hdr *trace;
+ struct ioam6_lwt *lwt;
+ struct nlattr *nla;
- if (nla_put_ioam6_trace(skb, IOAM6_IPTUNNEL_TRACE, trace))
+ nla = nla_reserve(skb, IOAM6_IPTUNNEL_TRACE, sizeof(*info));
+ if (!nla)
return -EMSGSIZE;
+ lwt = ioam6_lwt_state(lwtstate);
+ trace = ioam6_lwt_trace(lwtstate);
+
+ info = nla_data(nla);
+ info->mode = lwt->mode;
+ memcpy(&info->trace, trace, sizeof(*trace));
+ if (info->mode != IOAM6_IPTUNNEL_MODE_INLINE)
+ memcpy(&info->tundst, &lwt->tundst, sizeof(lwt->tundst));
+
return 0;
}
static int ioam6_encap_nlsize(struct lwtunnel_state *lwtstate)
{
- struct ioam6_trace_hdr *trace = ioam6_trace(lwtstate);
-
- return nla_total_size(sizeof(*trace));
+ return nla_total_size(sizeof(struct ioam6_iptunnel_trace));
}
static int ioam6_encap_cmp(struct lwtunnel_state *a, struct lwtunnel_state *b)
{
- struct ioam6_trace_hdr *a_hdr = ioam6_trace(a);
- struct ioam6_trace_hdr *b_hdr = ioam6_trace(b);
-
- return (a_hdr->namespace_id != b_hdr->namespace_id);
+ struct ioam6_trace_hdr *trace_a = ioam6_lwt_trace(a);
+ struct ioam6_trace_hdr *trace_b = ioam6_lwt_trace(b);
+ struct ioam6_lwt *lwt_a = ioam6_lwt_state(a);
+ struct ioam6_lwt *lwt_b = ioam6_lwt_state(b);
+
+ return (lwt_a->mode != lwt_b->mode ||
+ (lwt_a->mode != IOAM6_IPTUNNEL_MODE_INLINE &&
+ !ipv6_addr_equal(&lwt_a->tundst, &lwt_b->tundst)) ||
+ trace_a->namespace_id != trace_b->namespace_id);
}
static const struct lwtunnel_encap_ops ioam6_iptun_ops = {
- .build_state = ioam6_build_state,
+ .build_state = ioam6_build_state,
+ .destroy_state = ioam6_destroy_state,
.output = ioam6_output,
- .fill_encap = ioam6_fill_encap_info,
+ .fill_encap = ioam6_fill_encap_info,
.get_encap_size = ioam6_encap_nlsize,
- .cmp_encap = ioam6_encap_cmp,
- .owner = THIS_MODULE,
+ .cmp_encap = ioam6_encap_cmp,
+ .owner = THIS_MODULE,
};
int __init ioam6_iptunnel_init(void)
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next 2/2] selftests: net: Test for the IOAM encapsulation with IPv6
2021-09-28 19:03 [PATCH net-next 0/2] Support for the ip6ip6 encapsulation of IOAM Justin Iurman
2021-09-28 19:03 ` [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation Justin Iurman
@ 2021-09-28 19:03 ` Justin Iurman
2021-09-30 3:20 ` [PATCH net-next 0/2] Support for the ip6ip6 encapsulation of IOAM David Ahern
2 siblings, 0 replies; 11+ messages in thread
From: Justin Iurman @ 2021-09-28 19:03 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, yoshfuji, dsahern, justin.iurman
This patch adds support for testing the encap mode of IOAM.
Signed-off-by: Justin Iurman <justin.iurman@uliege.be>
---
tools/testing/selftests/net/ioam6.sh | 209 ++++++++++++++++++++-------
1 file changed, 159 insertions(+), 50 deletions(-)
diff --git a/tools/testing/selftests/net/ioam6.sh b/tools/testing/selftests/net/ioam6.sh
index 3caf72bb9c6a..90700303d8a9 100755
--- a/tools/testing/selftests/net/ioam6.sh
+++ b/tools/testing/selftests/net/ioam6.sh
@@ -6,7 +6,7 @@
# This script evaluates the IOAM insertion for IPv6 by checking the IOAM data
# consistency directly inside packets on the receiver side. Tests are divided
# into three categories: OUTPUT (evaluates the IOAM processing by the sender),
-# INPUT (evaluates the IOAM processing by the receiver) and GLOBAL (evaluates
+# INPUT (evaluates the IOAM processing by a receiver) and GLOBAL (evaluates
# wider use cases that do not fall into the other two categories). Both OUTPUT
# and INPUT tests only use a two-node topology (alpha and beta), while GLOBAL
# tests use the entire three-node topology (alpha, beta, gamma). Each test is
@@ -200,7 +200,7 @@ check_kernel_compatibility()
ip -netns ioam-tmp-node link set veth0 up
ip -netns ioam-tmp-node link set veth1 up
- ip -netns ioam-tmp-node ioam namespace add 0 &>/dev/null
+ ip -netns ioam-tmp-node ioam namespace add 0
ns_ad=$?
ip -netns ioam-tmp-node ioam namespace show | grep -q "namespace 0"
@@ -214,11 +214,11 @@ check_kernel_compatibility()
exit 1
fi
- ip -netns ioam-tmp-node route add db02::/64 encap ioam6 trace prealloc \
- type 0x800000 ns 0 size 4 dev veth0 &>/dev/null
+ ip -netns ioam-tmp-node route add db02::/64 encap ioam6 mode inline \
+ trace prealloc type 0x800000 ns 0 size 4 dev veth0
tr_ad=$?
- ip -netns ioam-tmp-node -6 route | grep -q "encap ioam6 trace"
+ ip -netns ioam-tmp-node -6 route | grep -q "encap ioam6"
tr_sh=$?
if [[ $tr_ad != 0 || $tr_sh != 0 ]]
@@ -232,6 +232,30 @@ check_kernel_compatibility()
ip link del veth0 2>/dev/null || true
ip netns del ioam-tmp-node || true
+
+ lsmod | grep -q "ip6_tunnel"
+ ip6tnl_loaded=$?
+
+ if [ $ip6tnl_loaded = 0 ]
+ then
+ encap_tests=0
+ else
+ modprobe ip6_tunnel &>/dev/null
+ lsmod | grep -q "ip6_tunnel"
+ encap_tests=$?
+
+ if [ $encap_tests != 0 ]
+ then
+ ip a | grep -q "ip6tnl0"
+ encap_tests=$?
+
+ if [ $encap_tests != 0 ]
+ then
+ echo "Note: ip6_tunnel not found neither as a module nor inside the" \
+ "kernel, tests that require it (encap mode) will be omitted"
+ fi
+ fi
+ fi
}
cleanup()
@@ -242,6 +266,11 @@ cleanup()
ip netns del ioam-node-alpha || true
ip netns del ioam-node-beta || true
ip netns del ioam-node-gamma || true
+
+ if [ $ip6tnl_loaded != 0 ]
+ then
+ modprobe -r ip6_tunnel 2>/dev/null || true
+ fi
}
setup()
@@ -329,6 +358,12 @@ log_test_failed()
printf "TEST: %-60s [FAIL]\n" "${desc}"
}
+log_results()
+{
+ echo "- Tests passed: ${npassed}"
+ echo "- Tests failed: ${nfailed}"
+}
+
run_test()
{
local name=$1
@@ -349,16 +384,26 @@ run_test()
ip netns exec $node_src ping6 -t 64 -c 1 -W 1 $ip6_dst &>/dev/null
if [ $? != 0 ]
then
+ nfailed=$((nfailed+1))
log_test_failed "${desc}"
kill -2 $spid &>/dev/null
else
wait $spid
- [ $? = 0 ] && log_test_passed "${desc}" || log_test_failed "${desc}"
+ if [ $? = 0 ]
+ then
+ npassed=$((npassed+1))
+ log_test_passed "${desc}"
+ else
+ nfailed=$((nfailed+1))
+ log_test_failed "${desc}"
+ fi
fi
}
run()
{
+ echo
+ printf "%0.s-" {1..74}
echo
echo "OUTPUT tests"
printf "%0.s-" {1..74}
@@ -369,7 +414,8 @@ run()
for t in $TESTS_OUTPUT
do
- $t
+ $t "inline"
+ [ $encap_tests = 0 ] && $t "encap"
done
# clean OUTPUT settings
@@ -377,6 +423,8 @@ run()
ip -netns ioam-node-alpha route change db01::/64 dev veth0
+ echo
+ printf "%0.s-" {1..74}
echo
echo "INPUT tests"
printf "%0.s-" {1..74}
@@ -387,7 +435,8 @@ run()
for t in $TESTS_INPUT
do
- $t
+ $t "inline"
+ [ $encap_tests = 0 ] && $t "encap"
done
# clean INPUT settings
@@ -396,7 +445,8 @@ run()
ip -netns ioam-node-alpha ioam namespace set 123 schema ${ALPHA[8]}
ip -netns ioam-node-alpha route change db01::/64 dev veth0
-
+ echo
+ printf "%0.s-" {1..74}
echo
echo "GLOBAL tests"
printf "%0.s-" {1..74}
@@ -404,8 +454,12 @@ run()
for t in $TESTS_GLOBAL
do
- $t
+ $t "inline"
+ [ $encap_tests = 0 ] && $t "encap"
done
+
+ echo
+ log_results
}
bit2type=(
@@ -431,11 +485,16 @@ out_undef_ns()
##############################################################################
local desc="Unknown IOAM namespace"
- ip -netns ioam-node-alpha route change db01::/64 encap ioam6 trace prealloc \
- type 0x800000 ns 0 size 4 dev veth0
+ [ "$1" = "encap" ] && mode="$1 tundst db01::1" || mode="$1"
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 up
+
+ ip -netns ioam-node-alpha route change db01::/64 encap ioam6 mode $mode \
+ trace prealloc type 0x800000 ns 0 size 4 dev veth0
- run_test ${FUNCNAME[0]} "${desc}" ioam-node-alpha ioam-node-beta db01::2 \
- db01::1 veth0 0x800000 0
+ run_test ${FUNCNAME[0]} "${desc} ($1 mode)" ioam-node-alpha ioam-node-beta \
+ db01::2 db01::1 veth0 0x800000 0
+
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 down
}
out_no_room()
@@ -446,11 +505,16 @@ out_no_room()
##############################################################################
local desc="Missing trace room"
- ip -netns ioam-node-alpha route change db01::/64 encap ioam6 trace prealloc \
- type 0xc00000 ns 123 size 4 dev veth0
+ [ "$1" = "encap" ] && mode="$1 tundst db01::1" || mode="$1"
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 up
+
+ ip -netns ioam-node-alpha route change db01::/64 encap ioam6 mode $mode \
+ trace prealloc type 0xc00000 ns 123 size 4 dev veth0
+
+ run_test ${FUNCNAME[0]} "${desc} ($1 mode)" ioam-node-alpha ioam-node-beta \
+ db01::2 db01::1 veth0 0xc00000 123
- run_test ${FUNCNAME[0]} "${desc}" ioam-node-alpha ioam-node-beta db01::2 \
- db01::1 veth0 0xc00000 123
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 down
}
out_bits()
@@ -465,15 +529,21 @@ out_bits()
local tmp=${bit2size[22]}
bit2size[22]=$(( $tmp + ${#ALPHA[9]} + ((4 - (${#ALPHA[9]} % 4)) % 4) ))
+ [ "$1" = "encap" ] && mode="$1 tundst db01::1" || mode="$1"
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 up
+
for i in {0..22}
do
- ip -netns ioam-node-alpha route change db01::/64 encap ioam6 trace \
- prealloc type ${bit2type[$i]} ns 123 size ${bit2size[$i]} dev veth0
+ ip -netns ioam-node-alpha route change db01::/64 encap ioam6 mode $mode \
+ trace prealloc type ${bit2type[$i]} ns 123 size ${bit2size[$i]} \
+ dev veth0
- run_test "out_bit$i" "${desc/<n>/$i}" ioam-node-alpha ioam-node-beta \
- db01::2 db01::1 veth0 ${bit2type[$i]} 123
+ run_test "out_bit$i" "${desc/<n>/$i} ($1 mode)" ioam-node-alpha \
+ ioam-node-beta db01::2 db01::1 veth0 ${bit2type[$i]} 123
done
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 down
+
bit2size[22]=$tmp
}
@@ -485,11 +555,16 @@ out_full_supp_trace()
##############################################################################
local desc="Full supported trace"
- ip -netns ioam-node-alpha route change db01::/64 encap ioam6 trace prealloc \
- type 0xfff002 ns 123 size 100 dev veth0
+ [ "$1" = "encap" ] && mode="$1 tundst db01::1" || mode="$1"
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 up
- run_test ${FUNCNAME[0]} "${desc}" ioam-node-alpha ioam-node-beta db01::2 \
- db01::1 veth0 0xfff002 123
+ ip -netns ioam-node-alpha route change db01::/64 encap ioam6 mode $mode \
+ trace prealloc type 0xfff002 ns 123 size 100 dev veth0
+
+ run_test ${FUNCNAME[0]} "${desc} ($1 mode)" ioam-node-alpha ioam-node-beta \
+ db01::2 db01::1 veth0 0xfff002 123
+
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 down
}
@@ -510,11 +585,16 @@ in_undef_ns()
##############################################################################
local desc="Unknown IOAM namespace"
- ip -netns ioam-node-alpha route change db01::/64 encap ioam6 trace prealloc \
- type 0x800000 ns 0 size 4 dev veth0
+ [ "$1" = "encap" ] && mode="$1 tundst db01::1" || mode="$1"
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 up
+
+ ip -netns ioam-node-alpha route change db01::/64 encap ioam6 mode $mode \
+ trace prealloc type 0x800000 ns 0 size 4 dev veth0
- run_test ${FUNCNAME[0]} "${desc}" ioam-node-alpha ioam-node-beta db01::2 \
- db01::1 veth0 0x800000 0
+ run_test ${FUNCNAME[0]} "${desc} ($1 mode)" ioam-node-alpha ioam-node-beta \
+ db01::2 db01::1 veth0 0x800000 0
+
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 down
}
in_no_room()
@@ -525,11 +605,16 @@ in_no_room()
##############################################################################
local desc="Missing trace room"
- ip -netns ioam-node-alpha route change db01::/64 encap ioam6 trace prealloc \
- type 0xc00000 ns 123 size 4 dev veth0
+ [ "$1" = "encap" ] && mode="$1 tundst db01::1" || mode="$1"
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 up
+
+ ip -netns ioam-node-alpha route change db01::/64 encap ioam6 mode $mode \
+ trace prealloc type 0xc00000 ns 123 size 4 dev veth0
- run_test ${FUNCNAME[0]} "${desc}" ioam-node-alpha ioam-node-beta db01::2 \
- db01::1 veth0 0xc00000 123
+ run_test ${FUNCNAME[0]} "${desc} ($1 mode)" ioam-node-alpha ioam-node-beta \
+ db01::2 db01::1 veth0 0xc00000 123
+
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 down
}
in_bits()
@@ -544,15 +629,21 @@ in_bits()
local tmp=${bit2size[22]}
bit2size[22]=$(( $tmp + ${#BETA[9]} + ((4 - (${#BETA[9]} % 4)) % 4) ))
+ [ "$1" = "encap" ] && mode="$1 tundst db01::1" || mode="$1"
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 up
+
for i in {0..22}
do
- ip -netns ioam-node-alpha route change db01::/64 encap ioam6 trace \
- prealloc type ${bit2type[$i]} ns 123 size ${bit2size[$i]} dev veth0
+ ip -netns ioam-node-alpha route change db01::/64 encap ioam6 mode $mode \
+ trace prealloc type ${bit2type[$i]} ns 123 size ${bit2size[$i]} \
+ dev veth0
- run_test "in_bit$i" "${desc/<n>/$i}" ioam-node-alpha ioam-node-beta \
- db01::2 db01::1 veth0 ${bit2type[$i]} 123
+ run_test "in_bit$i" "${desc/<n>/$i} ($1 mode)" ioam-node-alpha \
+ ioam-node-beta db01::2 db01::1 veth0 ${bit2type[$i]} 123
done
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 down
+
bit2size[22]=$tmp
}
@@ -569,11 +660,16 @@ in_oflag()
# back the IOAM namespace that was previously configured on the sender.
ip -netns ioam-node-alpha ioam namespace add 123
- ip -netns ioam-node-alpha route change db01::/64 encap ioam6 trace prealloc \
- type 0xc00000 ns 123 size 4 dev veth0
+ [ "$1" = "encap" ] && mode="$1 tundst db01::1" || mode="$1"
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 up
+
+ ip -netns ioam-node-alpha route change db01::/64 encap ioam6 mode $mode \
+ trace prealloc type 0xc00000 ns 123 size 4 dev veth0
+
+ run_test ${FUNCNAME[0]} "${desc} ($1 mode)" ioam-node-alpha ioam-node-beta \
+ db01::2 db01::1 veth0 0xc00000 123
- run_test ${FUNCNAME[0]} "${desc}" ioam-node-alpha ioam-node-beta db01::2 \
- db01::1 veth0 0xc00000 123
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 down
# And we clean the exception for this test to get things back to normal for
# other INPUT tests
@@ -588,11 +684,16 @@ in_full_supp_trace()
##############################################################################
local desc="Full supported trace"
- ip -netns ioam-node-alpha route change db01::/64 encap ioam6 trace prealloc \
- type 0xfff002 ns 123 size 80 dev veth0
+ [ "$1" = "encap" ] && mode="$1 tundst db01::1" || mode="$1"
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 up
- run_test ${FUNCNAME[0]} "${desc}" ioam-node-alpha ioam-node-beta db01::2 \
- db01::1 veth0 0xfff002 123
+ ip -netns ioam-node-alpha route change db01::/64 encap ioam6 mode $mode \
+ trace prealloc type 0xfff002 ns 123 size 80 dev veth0
+
+ run_test ${FUNCNAME[0]} "${desc} ($1 mode)" ioam-node-alpha ioam-node-beta \
+ db01::2 db01::1 veth0 0xfff002 123
+
+ [ "$1" = "encap" ] && ip -netns ioam-node-beta link set ip6tnl0 down
}
@@ -611,11 +712,16 @@ fwd_full_supp_trace()
##############################################################################
local desc="Forward - Full supported trace"
- ip -netns ioam-node-alpha route change db02::/64 encap ioam6 trace prealloc \
- type 0xfff002 ns 123 size 244 via db01::1 dev veth0
+ [ "$1" = "encap" ] && mode="$1 tundst db02::2" || mode="$1"
+ [ "$1" = "encap" ] && ip -netns ioam-node-gamma link set ip6tnl0 up
+
+ ip -netns ioam-node-alpha route change db02::/64 encap ioam6 mode $mode \
+ trace prealloc type 0xfff002 ns 123 size 244 via db01::1 dev veth0
- run_test ${FUNCNAME[0]} "${desc}" ioam-node-alpha ioam-node-gamma db01::2 \
- db02::2 veth0 0xfff002 123
+ run_test ${FUNCNAME[0]} "${desc} ($1 mode)" ioam-node-alpha ioam-node-gamma \
+ db01::2 db02::2 veth0 0xfff002 123
+
+ [ "$1" = "encap" ] && ip -netns ioam-node-gamma link set ip6tnl0 down
}
@@ -625,6 +731,9 @@ fwd_full_supp_trace()
# #
################################################################################
+npassed=0
+nfailed=0
+
if [ "$(id -u)" -ne 0 ]
then
echo "SKIP: Need root privileges"
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 0/2] Support for the ip6ip6 encapsulation of IOAM
2021-09-28 19:03 [PATCH net-next 0/2] Support for the ip6ip6 encapsulation of IOAM Justin Iurman
2021-09-28 19:03 ` [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation Justin Iurman
2021-09-28 19:03 ` [PATCH net-next 2/2] selftests: net: Test for the IOAM encapsulation with IPv6 Justin Iurman
@ 2021-09-30 3:20 ` David Ahern
2021-09-30 12:32 ` Justin Iurman
2 siblings, 1 reply; 11+ messages in thread
From: David Ahern @ 2021-09-30 3:20 UTC (permalink / raw)
To: Justin Iurman, netdev; +Cc: davem, kuba, yoshfuji, dsahern
On 9/28/21 1:03 PM, Justin Iurman wrote:
> In the current implementation, IOAM can only be inserted directly (i.e., only
> inside packets generated locally) by default, to be compliant with RFC8200.
>
> This patch adds support for in-transit packets and provides the ip6ip6
> encapsulation of IOAM. Therefore, three explicit encap modes are defined:
>
> - inline: directly inserts IOAM inside packets.
>
> - encap: ip6ip6 encapsulation of IOAM inside packets.
>
> - auto: either inline mode for packets generated locally or encap mode for
> in-transit packets.
>
> With current iproute2 implementation, it is configured this way:
>
> $ ip -6 r [...] encap ioam6 trace prealloc type 0x800000 ns 1 size 12 [...]
>
> Now, an encap mode must be specified:
>
> (inline mode)
> $ [...] encap ioam6 mode inline trace prealloc [...]
I take this to mean you want to change the CLI for ioam6? If so, that
does not happen once an iproute2 version has shipped with some previous
command line; it needs to be backwards compatible.
>
> (encap mode)
> $ [...] encap ioam6 mode encap tundst fc00::2 trace prealloc [...]
>
> (auto mode)
> $ [...] encap ioam6 mode auto tundst fc00::2 trace prealloc [...]
>
> A tunnel destination address must be configured when using the encap mode or the
> auto mode.
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation
2021-09-28 19:03 ` [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation Justin Iurman
@ 2021-09-30 3:26 ` David Ahern
2021-09-30 15:19 ` Justin Iurman
0 siblings, 1 reply; 11+ messages in thread
From: David Ahern @ 2021-09-30 3:26 UTC (permalink / raw)
To: Justin Iurman, netdev; +Cc: davem, kuba, yoshfuji, dsahern
On 9/28/21 1:03 PM, Justin Iurman wrote:
> @@ -42,34 +49,15 @@ static struct ioam6_lwt_encap *ioam6_lwt_info(struct lwtunnel_state *lwt)
> return &ioam6_lwt_state(lwt)->tuninfo;
> }
>
> -static struct ioam6_trace_hdr *ioam6_trace(struct lwtunnel_state *lwt)
> +static struct ioam6_trace_hdr *ioam6_lwt_trace(struct lwtunnel_state *lwt)
> {
> return &(ioam6_lwt_state(lwt)->tuninfo.traceh);
> }
>
> 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 */
+};
Also, no gaps in uapi. Make sure all holes are stated; an anonymous
entry is best.
> };
>
> -static int nla_put_ioam6_trace(struct sk_buff *skb, int attrtype,
> - struct ioam6_trace_hdr *trace)
> -{
> - struct ioam6_trace_hdr *data;
> - struct nlattr *nla;
> - int len;
> -
> - len = sizeof(*trace);
> -
> - nla = nla_reserve(skb, attrtype, len);
> - if (!nla)
> - return -EMSGSIZE;
> -
> - data = nla_data(nla);
> - memcpy(data, trace, len);
> -
> - return 0;
> -}
> -
quite a bit of the change seems like refactoring from existing feature
to allow the new ones. Please submit refactoring changes as a
prerequisite patch. The patch that introduces your new feature should be
focused solely on what is needed to implement that feature.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 0/2] Support for the ip6ip6 encapsulation of IOAM
2021-09-30 3:20 ` [PATCH net-next 0/2] Support for the ip6ip6 encapsulation of IOAM David Ahern
@ 2021-09-30 12:32 ` Justin Iurman
0 siblings, 0 replies; 11+ messages in thread
From: Justin Iurman @ 2021-09-30 12:32 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, davem, kuba, yoshfuji, dsahern
>> With current iproute2 implementation, it is configured this way:
>>
>> $ ip -6 r [...] encap ioam6 trace prealloc type 0x800000 ns 1 size 12 [...]
>>
>> Now, an encap mode must be specified:
>>
>> (inline mode)
>> $ [...] encap ioam6 mode inline trace prealloc [...]
>
> I take this to mean you want to change the CLI for ioam6? If so, that
> does not happen once an iproute2 version has shipped with some previous
> command line; it needs to be backwards compatible.
Sure. The inline mode would be the default one when using the old syntax (i.e., without specifying a mode).
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation
2021-09-30 3:26 ` David Ahern
@ 2021-09-30 15:19 ` Justin Iurman
2021-09-30 18:20 ` David Ahern
0 siblings, 1 reply; 11+ messages in thread
From: Justin Iurman @ 2021-09-30 15:19 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, davem, kuba, yoshfuji, dsahern
>> 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?
> 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;
};
};
>> };
>>
>> -static int nla_put_ioam6_trace(struct sk_buff *skb, int attrtype,
>> - struct ioam6_trace_hdr *trace)
>> -{
>> - struct ioam6_trace_hdr *data;
>> - struct nlattr *nla;
>> - int len;
>> -
>> - len = sizeof(*trace);
>> -
>> - nla = nla_reserve(skb, attrtype, len);
>> - if (!nla)
>> - return -EMSGSIZE;
>> -
>> - data = nla_data(nla);
>> - memcpy(data, trace, len);
>> -
>> - return 0;
>> -}
>> -
>
> quite a bit of the change seems like refactoring from existing feature
> to allow the new ones. Please submit refactoring changes as a
> prerequisite patch. The patch that introduces your new feature should be
> focused solely on what is needed to implement that feature.
+1, will do.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation
2021-09-30 15:19 ` Justin Iurman
@ 2021-09-30 18:20 ` David Ahern
2021-10-01 11:38 ` Justin Iurman
0 siblings, 1 reply; 11+ messages in thread
From: David Ahern @ 2021-09-30 18:20 UTC (permalink / raw)
To: Justin Iurman; +Cc: netdev, davem, kuba, yoshfuji, dsahern
On 9/30/21 9:19 AM, Justin Iurman wrote:
>>> 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.
>
>> 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).
>
>>> };
>>>
>>> -static int nla_put_ioam6_trace(struct sk_buff *skb, int attrtype,
>>> - struct ioam6_trace_hdr *trace)
>>> -{
>>> - struct ioam6_trace_hdr *data;
>>> - struct nlattr *nla;
>>> - int len;
>>> -
>>> - len = sizeof(*trace);
>>> -
>>> - nla = nla_reserve(skb, attrtype, len);
>>> - if (!nla)
>>> - return -EMSGSIZE;
>>> -
>>> - data = nla_data(nla);
>>> - memcpy(data, trace, len);
>>> -
>>> - return 0;
>>> -}
>>> -
>>
>> quite a bit of the change seems like refactoring from existing feature
>> to allow the new ones. Please submit refactoring changes as a
>> prerequisite patch. The patch that introduces your new feature should be
>> focused solely on what is needed to implement that feature.
>
> +1, will do.
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation
2021-09-30 18:20 ` David Ahern
@ 2021-10-01 11:38 ` Justin Iurman
2021-10-01 14:06 ` David Ahern
0 siblings, 1 reply; 11+ messages in thread
From: Justin Iurman @ 2021-10-01 11:38 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, davem, kuba, yoshfuji, dsahern
>>>> 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.
>>> 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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation
2021-10-01 11:38 ` Justin Iurman
@ 2021-10-01 14:06 ` David Ahern
2021-10-01 14:10 ` Justin Iurman
0 siblings, 1 reply; 11+ messages in thread
From: David Ahern @ 2021-10-01 14:06 UTC (permalink / raw)
To: Justin Iurman; +Cc: netdev, davem, kuba, yoshfuji, dsahern
On 10/1/21 5:38 AM, Justin Iurman wrote:
>>>>> 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.
ok, forgot about the data field.
Why not make the new data separate attributes then? Avoids the alignment
problem.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation
2021-10-01 14:06 ` David Ahern
@ 2021-10-01 14:10 ` Justin Iurman
0 siblings, 0 replies; 11+ messages in thread
From: Justin Iurman @ 2021-10-01 14:10 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, davem, kuba, yoshfuji, dsahern
>>>>>> 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.
>
> ok, forgot about the data field.
>
> Why not make the new data separate attributes then? Avoids the alignment
> problem.
Great idea, I'll do that.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-10-01 14:10 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28 19:03 [PATCH net-next 0/2] Support for the ip6ip6 encapsulation of IOAM Justin Iurman
2021-09-28 19:03 ` [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation Justin Iurman
2021-09-30 3:26 ` David Ahern
2021-09-30 15:19 ` Justin Iurman
2021-09-30 18:20 ` David Ahern
2021-10-01 11:38 ` Justin Iurman
2021-10-01 14:06 ` David Ahern
2021-10-01 14:10 ` Justin Iurman
2021-09-28 19:03 ` [PATCH net-next 2/2] selftests: net: Test for the IOAM encapsulation with IPv6 Justin Iurman
2021-09-30 3:20 ` [PATCH net-next 0/2] Support for the ip6ip6 encapsulation of IOAM David Ahern
2021-09-30 12:32 ` Justin Iurman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.