* [PATCH net-next v2 0/2] net: ipv6: ioam6: introduce tunsrc @ 2024-08-13 12:27 Justin Iurman 2024-08-13 12:27 ` [PATCH net-next v2 1/2] net: ipv6: ioam6: code alignment Justin Iurman 2024-08-13 12:27 ` [PATCH net-next v2 2/2] net: ipv6: ioam6: new feature tunsrc Justin Iurman 0 siblings, 2 replies; 5+ messages in thread From: Justin Iurman @ 2024-08-13 12:27 UTC (permalink / raw) To: netdev; +Cc: davem, dsahern, edumazet, kuba, pabeni, linux-kernel, justin.iurman This series introduces a new feature called "tunsrc" (just like seg6 already does). v2: - add links to performance result figures (see patch#2 description) - move the ipv6_addr_any() check out of the datapath Justin Iurman (2): net: ipv6: ioam6: code alignment net: ipv6: ioam6: new feature tunsrc include/uapi/linux/ioam6_iptunnel.h | 7 +++ net/ipv6/ioam6_iptunnel.c | 66 +++++++++++++++++++++++++---- 2 files changed, 65 insertions(+), 8 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net-next v2 1/2] net: ipv6: ioam6: code alignment 2024-08-13 12:27 [PATCH net-next v2 0/2] net: ipv6: ioam6: introduce tunsrc Justin Iurman @ 2024-08-13 12:27 ` Justin Iurman 2024-08-13 12:27 ` [PATCH net-next v2 2/2] net: ipv6: ioam6: new feature tunsrc Justin Iurman 1 sibling, 0 replies; 5+ messages in thread From: Justin Iurman @ 2024-08-13 12:27 UTC (permalink / raw) To: netdev; +Cc: davem, dsahern, edumazet, kuba, pabeni, linux-kernel, justin.iurman This patch prepares the next one by correcting the alignment of some lines. Signed-off-by: Justin Iurman <justin.iurman@uliege.be> --- net/ipv6/ioam6_iptunnel.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c index bf7120ecea1e..cd2522f04edf 100644 --- a/net/ipv6/ioam6_iptunnel.c +++ b/net/ipv6/ioam6_iptunnel.c @@ -43,7 +43,7 @@ struct ioam6_lwt { atomic_t pkt_cnt; u8 mode; struct in6_addr tundst; - struct ioam6_lwt_encap tuninfo; + struct ioam6_lwt_encap tuninfo; }; static const struct netlink_range_validation freq_range = { @@ -73,7 +73,8 @@ static const struct nla_policy ioam6_iptunnel_policy[IOAM6_IPTUNNEL_MAX + 1] = { IOAM6_IPTUNNEL_MODE_MIN, IOAM6_IPTUNNEL_MODE_MAX), [IOAM6_IPTUNNEL_DST] = NLA_POLICY_EXACT_LEN(sizeof(struct in6_addr)), - [IOAM6_IPTUNNEL_TRACE] = NLA_POLICY_EXACT_LEN(sizeof(struct ioam6_trace_hdr)), + [IOAM6_IPTUNNEL_TRACE] = NLA_POLICY_EXACT_LEN( + sizeof(struct ioam6_trace_hdr)), }; static bool ioam6_validate_trace_hdr(struct ioam6_trace_hdr *trace) @@ -459,9 +460,9 @@ static int ioam6_encap_cmp(struct lwtunnel_state *a, struct lwtunnel_state *b) static const struct lwtunnel_encap_ops ioam6_iptun_ops = { .build_state = ioam6_build_state, .destroy_state = ioam6_destroy_state, - .output = ioam6_output, + .output = ioam6_output, .fill_encap = ioam6_fill_encap_info, - .get_encap_size = ioam6_encap_nlsize, + .get_encap_size = ioam6_encap_nlsize, .cmp_encap = ioam6_encap_cmp, .owner = THIS_MODULE, }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net-next v2 2/2] net: ipv6: ioam6: new feature tunsrc 2024-08-13 12:27 [PATCH net-next v2 0/2] net: ipv6: ioam6: introduce tunsrc Justin Iurman 2024-08-13 12:27 ` [PATCH net-next v2 1/2] net: ipv6: ioam6: code alignment Justin Iurman @ 2024-08-13 12:27 ` Justin Iurman 2024-08-16 17:35 ` Jakub Kicinski 1 sibling, 1 reply; 5+ messages in thread From: Justin Iurman @ 2024-08-13 12:27 UTC (permalink / raw) To: netdev; +Cc: davem, dsahern, edumazet, kuba, pabeni, linux-kernel, justin.iurman This patch provides a new feature (i.e., "tunsrc") for the tunnel (i.e., "encap") mode of ioam6. Just like seg6 already does, except it is attached to a route. The "tunsrc" is optional: when not provided (by default), the automatic resolution is applied. Using "tunsrc" when possible has a benefit: performance. See the comparison: - before (= "encap" mode): https://ibb.co/bNCzvf7 - after (= "encap" mode with "tunsrc"): https://ibb.co/PT8L6yq Signed-off-by: Justin Iurman <justin.iurman@uliege.be> --- include/uapi/linux/ioam6_iptunnel.h | 7 ++++ net/ipv6/ioam6_iptunnel.c | 57 +++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/include/uapi/linux/ioam6_iptunnel.h b/include/uapi/linux/ioam6_iptunnel.h index 38f6a8fdfd34..6cdbd0da7ad8 100644 --- a/include/uapi/linux/ioam6_iptunnel.h +++ b/include/uapi/linux/ioam6_iptunnel.h @@ -50,6 +50,13 @@ enum { IOAM6_IPTUNNEL_FREQ_K, /* u32 */ IOAM6_IPTUNNEL_FREQ_N, /* u32 */ + /* Tunnel src address. + * For encap,auto modes. + * Optional (automatic if + * not provided). + */ + IOAM6_IPTUNNEL_SRC, /* struct in6_addr */ + __IOAM6_IPTUNNEL_MAX, }; diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c index cd2522f04edf..15c20e37809f 100644 --- a/net/ipv6/ioam6_iptunnel.c +++ b/net/ipv6/ioam6_iptunnel.c @@ -42,6 +42,8 @@ struct ioam6_lwt { struct ioam6_lwt_freq freq; atomic_t pkt_cnt; u8 mode; + bool has_tunsrc; + struct in6_addr tunsrc; struct in6_addr tundst; struct ioam6_lwt_encap tuninfo; }; @@ -72,6 +74,7 @@ static const struct nla_policy ioam6_iptunnel_policy[IOAM6_IPTUNNEL_MAX + 1] = { [IOAM6_IPTUNNEL_MODE] = NLA_POLICY_RANGE(NLA_U8, IOAM6_IPTUNNEL_MODE_MIN, IOAM6_IPTUNNEL_MODE_MAX), + [IOAM6_IPTUNNEL_SRC] = NLA_POLICY_EXACT_LEN(sizeof(struct in6_addr)), [IOAM6_IPTUNNEL_DST] = NLA_POLICY_EXACT_LEN(sizeof(struct in6_addr)), [IOAM6_IPTUNNEL_TRACE] = NLA_POLICY_EXACT_LEN( sizeof(struct ioam6_trace_hdr)), @@ -144,6 +147,11 @@ static int ioam6_build_state(struct net *net, struct nlattr *nla, else mode = nla_get_u8(tb[IOAM6_IPTUNNEL_MODE]); + if (tb[IOAM6_IPTUNNEL_SRC] && mode == IOAM6_IPTUNNEL_MODE_INLINE) { + NL_SET_ERR_MSG(extack, "no tunnel src expected in inline mode"); + return -EINVAL; + } + if (!tb[IOAM6_IPTUNNEL_DST] && mode != IOAM6_IPTUNNEL_MODE_INLINE) { NL_SET_ERR_MSG(extack, "this mode needs a tunnel destination"); return -EINVAL; @@ -178,6 +186,23 @@ static int ioam6_build_state(struct net *net, struct nlattr *nla, ilwt->freq.n = freq_n; ilwt->mode = mode; + + if (!tb[IOAM6_IPTUNNEL_SRC]) { + ilwt->has_tunsrc = false; + } else { + ilwt->has_tunsrc = true; + ilwt->tunsrc = nla_get_in6_addr(tb[IOAM6_IPTUNNEL_SRC]); + + if (ipv6_addr_any(&ilwt->tunsrc)) { + dst_cache_destroy(&ilwt->cache); + kfree(lwt); + + NL_SET_ERR_MSG_ATTR(extack, tb[IOAM6_IPTUNNEL_SRC], + "invalid tunnel source address"); + return -EINVAL; + } + } + if (tb[IOAM6_IPTUNNEL_DST]) ilwt->tundst = nla_get_in6_addr(tb[IOAM6_IPTUNNEL_DST]); @@ -257,6 +282,8 @@ static int ioam6_do_inline(struct net *net, struct sk_buff *skb, static int ioam6_do_encap(struct net *net, struct sk_buff *skb, struct ioam6_lwt_encap *tuninfo, + bool has_tunsrc, + struct in6_addr *tunsrc, struct in6_addr *tundst) { struct dst_entry *dst = skb_dst(skb); @@ -286,8 +313,13 @@ static int ioam6_do_encap(struct net *net, struct sk_buff *skb, 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); + + if (has_tunsrc) { + memcpy(&hdr->saddr, tunsrc, sizeof(*tunsrc)); + } else { + ipv6_dev_get_saddr(net, dst->dev, &hdr->daddr, + IPV6_PREFER_SRC_PUBLIC, &hdr->saddr); + } skb_postpush_rcsum(skb, hdr, len); @@ -329,7 +361,9 @@ static int ioam6_output(struct net *net, struct sock *sk, struct sk_buff *skb) case IOAM6_IPTUNNEL_MODE_ENCAP: do_encap: /* Encapsulation (ip6ip6) */ - err = ioam6_do_encap(net, skb, &ilwt->tuninfo, &ilwt->tundst); + err = ioam6_do_encap(net, skb, &ilwt->tuninfo, + ilwt->has_tunsrc, &ilwt->tunsrc, + &ilwt->tundst); if (unlikely(err)) goto drop; @@ -415,6 +449,13 @@ static int ioam6_fill_encap_info(struct sk_buff *skb, goto ret; if (ilwt->mode != IOAM6_IPTUNNEL_MODE_INLINE) { + if (ilwt->has_tunsrc) { + err = nla_put_in6_addr(skb, IOAM6_IPTUNNEL_SRC, + &ilwt->tunsrc); + if (err) + goto ret; + } + err = nla_put_in6_addr(skb, IOAM6_IPTUNNEL_DST, &ilwt->tundst); if (err) goto ret; @@ -436,8 +477,12 @@ static int ioam6_encap_nlsize(struct lwtunnel_state *lwtstate) nla_total_size(sizeof(ilwt->mode)) + nla_total_size(sizeof(ilwt->tuninfo.traceh)); - if (ilwt->mode != IOAM6_IPTUNNEL_MODE_INLINE) + if (ilwt->mode != IOAM6_IPTUNNEL_MODE_INLINE) { + if (ilwt->has_tunsrc) + nlsize += nla_total_size(sizeof(ilwt->tunsrc)); + nlsize += nla_total_size(sizeof(ilwt->tundst)); + } return nlsize; } @@ -452,8 +497,12 @@ static int ioam6_encap_cmp(struct lwtunnel_state *a, struct lwtunnel_state *b) 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->has_tunsrc != ilwt_b->has_tunsrc || (ilwt_a->mode != IOAM6_IPTUNNEL_MODE_INLINE && !ipv6_addr_equal(&ilwt_a->tundst, &ilwt_b->tundst)) || + (ilwt_a->mode != IOAM6_IPTUNNEL_MODE_INLINE && + ilwt_a->has_tunsrc && + !ipv6_addr_equal(&ilwt_a->tunsrc, &ilwt_b->tunsrc)) || trace_a->namespace_id != trace_b->namespace_id); } -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v2 2/2] net: ipv6: ioam6: new feature tunsrc 2024-08-13 12:27 ` [PATCH net-next v2 2/2] net: ipv6: ioam6: new feature tunsrc Justin Iurman @ 2024-08-16 17:35 ` Jakub Kicinski 2024-08-17 12:57 ` Justin Iurman 0 siblings, 1 reply; 5+ messages in thread From: Jakub Kicinski @ 2024-08-16 17:35 UTC (permalink / raw) To: Justin Iurman; +Cc: netdev, davem, dsahern, edumazet, pabeni, linux-kernel On Tue, 13 Aug 2024 14:27:23 +0200 Justin Iurman wrote: > This patch provides a new feature (i.e., "tunsrc") for the tunnel (i.e., > "encap") mode of ioam6. Just like seg6 already does, except it is > attached to a route. The "tunsrc" is optional: when not provided (by > default), the automatic resolution is applied. Using "tunsrc" when > possible has a benefit: performance. See the comparison: > - before (= "encap" mode): https://ibb.co/bNCzvf7 > - after (= "encap" mode with "tunsrc"): https://ibb.co/PT8L6yq No need to extend the selftests ? > diff --git a/include/uapi/linux/ioam6_iptunnel.h b/include/uapi/linux/ioam6_iptunnel.h > index 38f6a8fdfd34..6cdbd0da7ad8 100644 > --- a/include/uapi/linux/ioam6_iptunnel.h > +++ b/include/uapi/linux/ioam6_iptunnel.h > @@ -50,6 +50,13 @@ enum { > IOAM6_IPTUNNEL_FREQ_K, /* u32 */ > IOAM6_IPTUNNEL_FREQ_N, /* u32 */ > > + /* Tunnel src address. > + * For encap,auto modes. > + * Optional (automatic if > + * not provided). The wrapping of this text appears excessive > + */ > + IOAM6_IPTUNNEL_SRC, /* struct in6_addr */ > + > __IOAM6_IPTUNNEL_MAX, > }; > @@ -178,6 +186,23 @@ static int ioam6_build_state(struct net *net, struct nlattr *nla, > ilwt->freq.n = freq_n; > > ilwt->mode = mode; > + > + if (!tb[IOAM6_IPTUNNEL_SRC]) { > + ilwt->has_tunsrc = false; > + } else { > + ilwt->has_tunsrc = true; > + ilwt->tunsrc = nla_get_in6_addr(tb[IOAM6_IPTUNNEL_SRC]); > + > + if (ipv6_addr_any(&ilwt->tunsrc)) { > + dst_cache_destroy(&ilwt->cache); > + kfree(lwt); Let's put the cleanup at the end of the function, and use a goto / label to jump there. > + NL_SET_ERR_MSG_ATTR(extack, tb[IOAM6_IPTUNNEL_SRC], > + "invalid tunnel source address"); > + return -EINVAL; > + } > + } > + > if (tb[IOAM6_IPTUNNEL_DST]) > ilwt->tundst = nla_get_in6_addr(tb[IOAM6_IPTUNNEL_DST]); > > @@ -257,6 +282,8 @@ static int ioam6_do_inline(struct net *net, struct sk_buff *skb, > > static int ioam6_do_encap(struct net *net, struct sk_buff *skb, > struct ioam6_lwt_encap *tuninfo, > + bool has_tunsrc, > + struct in6_addr *tunsrc, > struct in6_addr *tundst) > { > struct dst_entry *dst = skb_dst(skb); > @@ -286,8 +313,13 @@ static int ioam6_do_encap(struct net *net, struct sk_buff *skb, > 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); > + > + if (has_tunsrc) { > + memcpy(&hdr->saddr, tunsrc, sizeof(*tunsrc)); > + } else { > + ipv6_dev_get_saddr(net, dst->dev, &hdr->daddr, > + IPV6_PREFER_SRC_PUBLIC, &hdr->saddr); > + } single statement branches, no need for {} > skb_postpush_rcsum(skb, hdr, len); > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v2 2/2] net: ipv6: ioam6: new feature tunsrc 2024-08-16 17:35 ` Jakub Kicinski @ 2024-08-17 12:57 ` Justin Iurman 0 siblings, 0 replies; 5+ messages in thread From: Justin Iurman @ 2024-08-17 12:57 UTC (permalink / raw) To: Jakub Kicinski Cc: netdev, davem, dsahern, edumazet, pabeni, linux-kernel, justin.iurman On 8/16/24 19:35, Jakub Kicinski wrote: > On Tue, 13 Aug 2024 14:27:23 +0200 Justin Iurman wrote: >> This patch provides a new feature (i.e., "tunsrc") for the tunnel (i.e., >> "encap") mode of ioam6. Just like seg6 already does, except it is >> attached to a route. The "tunsrc" is optional: when not provided (by >> default), the automatic resolution is applied. Using "tunsrc" when >> possible has a benefit: performance. See the comparison: >> - before (= "encap" mode): https://ibb.co/bNCzvf7 >> - after (= "encap" mode with "tunsrc"): https://ibb.co/PT8L6yq > > No need to extend the selftests ? Hi Jakub, I've been wondering too... Currently, it doesn't check the IPv6 header and only focuses on the IOAM header+data. So I came to the conclusion that the selftests should not necessarily be updated for that. Although I'm not closing the door to a future update to include some extra IPv6 header checks. I just think it's not required *now* and in this series. I'll post -v3 to address your comments below (thanks, by the way!). Cheers, Justin >> diff --git a/include/uapi/linux/ioam6_iptunnel.h b/include/uapi/linux/ioam6_iptunnel.h >> index 38f6a8fdfd34..6cdbd0da7ad8 100644 >> --- a/include/uapi/linux/ioam6_iptunnel.h >> +++ b/include/uapi/linux/ioam6_iptunnel.h >> @@ -50,6 +50,13 @@ enum { >> IOAM6_IPTUNNEL_FREQ_K, /* u32 */ >> IOAM6_IPTUNNEL_FREQ_N, /* u32 */ >> >> + /* Tunnel src address. >> + * For encap,auto modes. >> + * Optional (automatic if >> + * not provided). > > The wrapping of this text appears excessive > >> + */ >> + IOAM6_IPTUNNEL_SRC, /* struct in6_addr */ >> + >> __IOAM6_IPTUNNEL_MAX, >> }; > >> @@ -178,6 +186,23 @@ static int ioam6_build_state(struct net *net, struct nlattr *nla, >> ilwt->freq.n = freq_n; >> >> ilwt->mode = mode; >> + >> + if (!tb[IOAM6_IPTUNNEL_SRC]) { >> + ilwt->has_tunsrc = false; >> + } else { >> + ilwt->has_tunsrc = true; >> + ilwt->tunsrc = nla_get_in6_addr(tb[IOAM6_IPTUNNEL_SRC]); >> + >> + if (ipv6_addr_any(&ilwt->tunsrc)) { >> + dst_cache_destroy(&ilwt->cache); >> + kfree(lwt); > > Let's put the cleanup at the end of the function, and use a goto / label > to jump there. > >> + NL_SET_ERR_MSG_ATTR(extack, tb[IOAM6_IPTUNNEL_SRC], >> + "invalid tunnel source address"); >> + return -EINVAL; >> + } >> + } >> + >> if (tb[IOAM6_IPTUNNEL_DST]) >> ilwt->tundst = nla_get_in6_addr(tb[IOAM6_IPTUNNEL_DST]); >> >> @@ -257,6 +282,8 @@ static int ioam6_do_inline(struct net *net, struct sk_buff *skb, >> >> static int ioam6_do_encap(struct net *net, struct sk_buff *skb, >> struct ioam6_lwt_encap *tuninfo, >> + bool has_tunsrc, >> + struct in6_addr *tunsrc, >> struct in6_addr *tundst) >> { >> struct dst_entry *dst = skb_dst(skb); >> @@ -286,8 +313,13 @@ static int ioam6_do_encap(struct net *net, struct sk_buff *skb, >> 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); >> + >> + if (has_tunsrc) { >> + memcpy(&hdr->saddr, tunsrc, sizeof(*tunsrc)); >> + } else { >> + ipv6_dev_get_saddr(net, dst->dev, &hdr->daddr, >> + IPV6_PREFER_SRC_PUBLIC, &hdr->saddr); >> + } > > single statement branches, no need for {} > >> skb_postpush_rcsum(skb, hdr, len); >> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-08-17 12:57 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-13 12:27 [PATCH net-next v2 0/2] net: ipv6: ioam6: introduce tunsrc Justin Iurman 2024-08-13 12:27 ` [PATCH net-next v2 1/2] net: ipv6: ioam6: code alignment Justin Iurman 2024-08-13 12:27 ` [PATCH net-next v2 2/2] net: ipv6: ioam6: new feature tunsrc Justin Iurman 2024-08-16 17:35 ` Jakub Kicinski 2024-08-17 12:57 ` Justin Iurman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).