On Mon, Nov 13, 2017 at 05:03:26PM +0800, Hangbin Liu wrote:
> geneve->sock4/6 were added with geneve_open and released with geneve_stop.
> So when geneve link down, we will not able to show remote address and
> checksum info after commit 11387fe4a98 ("geneve: fix fill_info when using
> collect_metadata").
>
> Fix this by reset back to the previous behavior: only show the corresponding
> remote address and always show checksum info.
Thanks Hangbin,
I think we can do better though and maybe avoid another round of this
churn.
>
> Fixes: 11387fe4a98 ("geneve: fix fill_info when using collect_metadata")
> Signed-off-by: Hangbin Liu <liuhang...@gmail.com>
> ---
> drivers/net/geneve.c | 28 +++++++++++-----------------
> 1 file changed, 11 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index ed51018..b010db7 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -1511,32 +1511,18 @@ static int geneve_fill_info(struct sk_buff *skb,
> const struct net_device *dev)
> if (nla_put_u32(skb, IFLA_GENEVE_ID, vni))
> goto nla_put_failure;
>
> - if (rtnl_dereference(geneve->sock4)) {
> + if (ip_tunnel_info_af(info) == AF_INET) {
> if (nla_put_in_addr(skb, IFLA_GENEVE_REMOTE,
> info->key.u.ipv4.dst))
> goto nla_put_failure;
>
We can avoid passing up *_REMOTE{,6} for COLLECT_METADATA. They're
mutually exclusive.
> - if (nla_put_u8(skb, IFLA_GENEVE_UDP_CSUM,
> - !!(info->key.tun_flags & TUNNEL_CSUM)))
> - goto nla_put_failure;
> -
> - }
> -
> #if IS_ENABLED(CONFIG_IPV6)
> - if (rtnl_dereference(geneve->sock6)) {
> + } else {
> if (nla_put_in6_addr(skb, IFLA_GENEVE_REMOTE6,
> &info->key.u.ipv6.dst))
> goto nla_put_failure;
> -
> - if (nla_put_u8(skb, IFLA_GENEVE_UDP_ZERO_CSUM6_TX,
> - !(info->key.tun_flags & TUNNEL_CSUM)))
> - goto nla_put_failure;
> -
> - if (nla_put_u8(skb, IFLA_GENEVE_UDP_ZERO_CSUM6_RX,
> - !geneve->use_udp6_rx_checksums))
> - goto nla_put_failure;
> - }
> #endif
> + }
>
> if (nla_put_u8(skb, IFLA_GENEVE_TTL, info->key.ttl) ||
> nla_put_u8(skb, IFLA_GENEVE_TOS, info->key.tos) ||
> @@ -1550,6 +1536,14 @@ static int geneve_fill_info(struct sk_buff *skb, const
> struct net_device *dev)
> if (nla_put_flag(skb, IFLA_GENEVE_COLLECT_METADATA))
> goto nla_put_failure;
> }
> +
> + if (nla_put_u8(skb, IFLA_GENEVE_UDP_CSUM,
> + !!(info->key.tun_flags & TUNNEL_CSUM)) ||
> + nla_put_u8(skb, IFLA_GENEVE_UDP_ZERO_CSUM6_TX,
> + !(info->key.tun_flags & TUNNEL_CSUM)) ||
These two are nonsensical for COLLECT_METADATA as they come from the skb
tun_info. They can be moved to inside the ipv4/ipv6 checks. For
non-metadata, it doesn't make sense to pass them both as the tunnel is
either ipv4 or ipv6, but not both.
> + nla_put_u8(skb, IFLA_GENEVE_UDP_ZERO_CSUM6_RX,
> + !geneve->use_udp6_rx_checksums))
> + goto nla_put_failure;
> return 0;
>
> nla_put_failure:
