From: Eric Dumazet <edumazet@google.com>
To: Alce Lafranque <alce@lafranque.net>
Cc: "David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Paolo Abeni <pabeni@redhat.com>,
David Ahern <dsahern@kernel.org>,
Ido Schimmel <idosch@nvidia.com>,
netdev@vger.kernel.org, vincent@bernat.ch
Subject: Re: [PATCH net-next] vxlan: add support for flowlabel inherit
Date: Sat, 30 Sep 2023 17:29:50 +0200 [thread overview]
Message-ID: <CANn89i+q_0e3ztiHD5YE4LBJCSeaETk3VyJ0TPuJYP9By1_1Tg@mail.gmail.com> (raw)
In-Reply-To: <4444C5AE-FA5A-49A4-9700-7DD9D7916C0F.1@mail.lac-coloc.fr>
On Sat, Sep 30, 2023 at 5:13 PM Alce Lafranque <alce@lafranque.net> wrote:
>
> By default, VXLAN encapsulation over IPv6 sets the flow label to 0, with an
> option for a fixed value. This commits add the ability to inherit the flow
> label from the inner packet, like for other tunnel implementations.
>
> ```
> $ ./ip/ip addr add 2001:db8::2/64 dev dummy1
> $ ./ip/ip link set up dev dummy1
> $ ./ip/ip link add vxlan1 type vxlan id 100 flowlabel inherit remote 2001:db8::1 local 2001:db8::2
Side question : How can "flowlabel inherit" can be turned off later
with an "ip link change ..." ?
It seems vxlan_nl2flag() would always turn it 'on' for NLA_FLAG type :
if (vxlan_policy[attrtype].type == NLA_FLAG)
flags = conf->flags | mask; // always turn on
else if (nla_get_u8(tb[attrtype])) // dead code for NLA_FLAG
flags = conf->flags | mask;
else
flags = conf->flags & ~mask;
conf->flags = flags;
> $ ./ip/ip link set up dev vxlan1
> $ ./ip/ip addr add 2001:db8:1::2/64 dev vxlan1
> $ ./ip/ip link set arp off dev vxlan1
> $ ping -q 2001:db8:1::1 &
> $ tshark -d udp.port==8472,vxlan -Vpni dummy1 -c1
> [...]
> Internet Protocol Version 6, Src: 2001:db8::2, Dst: 2001:db8::1
> 0110 .... = Version: 6
> .... 0000 0000 .... .... .... .... .... = Traffic Class: 0x00 (DSCP: CS0, ECN: Not-ECT)
> .... 0000 00.. .... .... .... .... .... = Differentiated Services Codepoint: Default (0)
> .... .... ..00 .... .... .... .... .... = Explicit Congestion Notification: Not ECN-Capable Transport (0)
> .... 1011 0001 1010 1111 1011 = Flow Label: 0xb1afb
> [...]
> Virtual eXtensible Local Area Network
> Flags: 0x0800, VXLAN Network ID (VNI)
> Group Policy ID: 0
> VXLAN Network Identifier (VNI): 100
> [...]
> Internet Protocol Version 6, Src: 2001:db8:1::2, Dst: 2001:db8:1::1
> 0110 .... = Version: 6
> .... 0000 0000 .... .... .... .... .... = Traffic Class: 0x00 (DSCP: CS0, ECN: Not-ECT)
> .... 0000 00.. .... .... .... .... .... = Differentiated Services Codepoint: Default (0)
> .... .... ..00 .... .... .... .... .... = Explicit Congestion Notification: Not ECN-Capable Transport (0)
> .... 1011 0001 1010 1111 1011 = Flow Label: 0xb1afb
> ```
>
> Signed-off-by: Alce Lafranque <alce@lafranque.net>
> Co-developed-by: Vincent Bernat <vincent@bernat.ch>
> Signed-off-by: Vincent Bernat <vincent@bernat.ch>
> ---
> drivers/net/vxlan/vxlan_core.c | 20 ++++++++++++++++++--
> include/net/ip_tunnels.h | 11 +++++++++++
> include/net/vxlan.h | 2 ++
> include/uapi/linux/if_link.h | 1 +
> 4 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index 5b5597073b00..aa7fbfdd93b1 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -2475,7 +2475,11 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
> else
> udp_sum = !(flags & VXLAN_F_UDP_ZERO_CSUM6_TX);
> #if IS_ENABLED(CONFIG_IPV6)
> - label = vxlan->cfg.label;
> + if (flags & VXLAN_F_LABEL_INHERIT) {
> + label = ip_tunnel_get_flowlabel(old_iph, skb);
> + } else {
> + label = vxlan->cfg.label;
> + }
You can remove the braces.
> #endif
> } else {
> if (!info) {
> @@ -3286,6 +3290,7 @@ static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
> [IFLA_VXLAN_DF] = { .type = NLA_U8 },
> [IFLA_VXLAN_VNIFILTER] = { .type = NLA_U8 },
> [IFLA_VXLAN_LOCALBYPASS] = NLA_POLICY_MAX(NLA_U8, 1),
> + [IFLA_VXLAN_LABEL_INHERIT] = { .type = NLA_FLAG },
> };
>
> static int vxlan_validate(struct nlattr *tb[], struct nlattr *data[],
> @@ -4001,7 +4006,15 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
>
> if (data[IFLA_VXLAN_LABEL])
> conf->label = nla_get_be32(data[IFLA_VXLAN_LABEL]) &
> - IPV6_FLOWLABEL_MASK;
> + IPV6_FLOWLABEL_MASK;
> +
> + if (data[IFLA_VXLAN_LABEL_INHERIT]) {
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_LABEL_INHERIT,
> + VXLAN_F_LABEL_INHERIT, changelink, false,
> + extack);
> + if (err)
> + return err;
> + }
>
> if (data[IFLA_VXLAN_LEARNING]) {
> err = vxlan_nl2flag(conf, data, IFLA_VXLAN_LEARNING,
> @@ -4315,6 +4328,7 @@ static size_t vxlan_get_size(const struct net_device *dev)
> nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_TOS */
> nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_DF */
> nla_total_size(sizeof(__be32)) + /* IFLA_VXLAN_LABEL */
> + nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_LABEL_INHERIT */
> nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_LEARNING */
> nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_PROXY */
> nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_RSC */
> @@ -4387,6 +4401,8 @@ static int vxlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
> nla_put_u8(skb, IFLA_VXLAN_TOS, vxlan->cfg.tos) ||
> nla_put_u8(skb, IFLA_VXLAN_DF, vxlan->cfg.df) ||
> nla_put_be32(skb, IFLA_VXLAN_LABEL, vxlan->cfg.label) ||
> + nla_put_u8(skb, IFLA_VXLAN_LABEL_INHERIT,
> + !!(vxlan->cfg.flags & VXLAN_F_LABEL_INHERIT)) ||
This seems in contradiction with NLA_FLAG semantics if the flag can
not be turned off.
Look for nla_put_flag(). User space could get confused.
> nla_put_u8(skb, IFLA_VXLAN_LEARNING,
> !!(vxlan->cfg.flags & VXLAN_F_LEARN)) ||
> nla_put_u8(skb, IFLA_VXLAN_PROXY,
>
next prev parent reply other threads:[~2023-09-30 15:30 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-30 14:28 [PATCH net-next] vxlan: add support for flowlabel inherit Alce Lafranque
2023-09-30 15:29 ` Eric Dumazet [this message]
2023-09-30 18:16 ` alce
2023-10-01 21:21 ` Vincent Bernat
2023-10-03 10:59 ` Ido Schimmel
2023-10-07 14:46 ` alce
2023-10-07 14:26 ` [PATCH net-next v2] " Alce Lafranque
2023-10-07 15:44 ` kernel test robot
2023-10-11 7:11 ` Ido Schimmel
2023-10-07 17:09 ` [PATCH net-next] " Tom Herbert
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CANn89i+q_0e3ztiHD5YE4LBJCSeaETk3VyJ0TPuJYP9By1_1Tg@mail.gmail.com \
--to=edumazet@google.com \
--cc=alce@lafranque.net \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=idosch@nvidia.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=vincent@bernat.ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).