[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANn89i+q_0e3ztiHD5YE4LBJCSeaETk3VyJ0TPuJYP9By1_1Tg@mail.gmail.com>
Date: Sat, 30 Sep 2023 17:29:50 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Alce Lafranque <alce@...ranque.net>
Cc: "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, David Ahern <dsahern@...nel.org>, Ido Schimmel <idosch@...dia.com>,
netdev@...r.kernel.org, vincent@...nat.ch
Subject: Re: [PATCH net-next] vxlan: add support for flowlabel inherit
On Sat, Sep 30, 2023 at 5:13 PM Alce Lafranque <alce@...ranque.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@...ranque.net>
> Co-developed-by: Vincent Bernat <vincent@...nat.ch>
> Signed-off-by: Vincent Bernat <vincent@...nat.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,
>
Powered by blists - more mailing lists