* [PATCH net] geneve: show remote address and checksum info even after link down
@ 2017-11-13 9:03 Hangbin Liu
2017-11-13 20:27 ` Eric Garver
2017-11-15 1:43 ` [PATCHv2 net] geneve: fix fill_info when " Hangbin Liu
0 siblings, 2 replies; 6+ messages in thread
From: Hangbin Liu @ 2017-11-13 9:03 UTC (permalink / raw)
To: netdev
Cc: Eric Garver, Pravin B Shelar, David S. Miller, Jiri Benc,
Hangbin Liu
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.
Fixes: 11387fe4a98 ("geneve: fix fill_info when using collect_metadata")
Signed-off-by: Hangbin Liu <liuhangbin@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;
- 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)) ||
+ nla_put_u8(skb, IFLA_GENEVE_UDP_ZERO_CSUM6_RX,
+ !geneve->use_udp6_rx_checksums))
+ goto nla_put_failure;
return 0;
nla_put_failure:
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net] geneve: show remote address and checksum info even after link down
2017-11-13 9:03 [PATCH net] geneve: show remote address and checksum info even after link down Hangbin Liu
@ 2017-11-13 20:27 ` Eric Garver
2017-11-14 2:39 ` Hangbin Liu
2017-11-15 1:43 ` [PATCHv2 net] geneve: fix fill_info when " Hangbin Liu
1 sibling, 1 reply; 6+ messages in thread
From: Eric Garver @ 2017-11-13 20:27 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Pravin B Shelar, David S. Miller, Jiri Benc
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 <liuhangbin@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:
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] geneve: show remote address and checksum info even after link down
2017-11-13 20:27 ` Eric Garver
@ 2017-11-14 2:39 ` Hangbin Liu
2017-11-14 15:48 ` Eric Garver
0 siblings, 1 reply; 6+ messages in thread
From: Hangbin Liu @ 2017-11-14 2:39 UTC (permalink / raw)
To: Eric Garver, Hangbin Liu, netdev, Pravin B Shelar,
David S. Miller, Jiri Benc
Hi Eric,
Thanks for the comments.
On Mon, Nov 13, 2017 at 03:27:25PM -0500, Eric Garver wrote:
> > Fixes: 11387fe4a98 ("geneve: fix fill_info when using collect_metadata")
> > Signed-off-by: Hangbin Liu <liuhangbin@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.
Do you mean something like
+ bool metadata = geneve->collect_md;
- if (rtnl_dereference(geneve->sock4)) {
+ if (!metadata && ip_tunnel_info_af(info) == AF_INET) {
>
> > - 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 {
and
- if (rtnl_dereference(geneve->sock6)) {
+ } else if (!metadata) {
?
> > +
> > + 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.
OK, I will put them back to the ipv4/6 chunks.
Thanks
Hangbin
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] geneve: show remote address and checksum info even after link down
2017-11-14 2:39 ` Hangbin Liu
@ 2017-11-14 15:48 ` Eric Garver
0 siblings, 0 replies; 6+ messages in thread
From: Eric Garver @ 2017-11-14 15:48 UTC (permalink / raw)
To: Hangbin Liu
Cc: Hangbin Liu, netdev, Pravin B Shelar, David S. Miller, Jiri Benc
On Tue, Nov 14, 2017 at 10:39:41AM +0800, Hangbin Liu wrote:
> Hi Eric,
>
> Thanks for the comments.
>
> On Mon, Nov 13, 2017 at 03:27:25PM -0500, Eric Garver wrote:
> > > Fixes: 11387fe4a98 ("geneve: fix fill_info when using collect_metadata")
> > > Signed-off-by: Hangbin Liu <liuhangbin@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.
>
> Do you mean something like
>
> + bool metadata = geneve->collect_md;
> - if (rtnl_dereference(geneve->sock4)) {
> + if (!metadata && ip_tunnel_info_af(info) == AF_INET) {
Yes.
> >
> > > - 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 {
>
> and
>
> - if (rtnl_dereference(geneve->sock6)) {
> + } else if (!metadata) {
>
> ?
Yes.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCHv2 net] geneve: fix fill_info when link down
2017-11-13 9:03 [PATCH net] geneve: show remote address and checksum info even after link down Hangbin Liu
2017-11-13 20:27 ` Eric Garver
@ 2017-11-15 1:43 ` Hangbin Liu
2017-11-15 10:48 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Hangbin Liu @ 2017-11-15 1:43 UTC (permalink / raw)
To: netdev
Cc: Eric Garver, Pravin B Shelar, David S. Miller, Jiri Benc,
Hangbin Liu
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 avoid passing *_REMOTE{,6} for COLLECT_METADATA since they are
mutually exclusive, and always show UDP_ZERO_CSUM6_RX info.
Fixes: 11387fe4a98 ("geneve: fix fill_info when using collect_metadata")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
drivers/net/geneve.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index ed51018..b9d8d71 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1503,6 +1503,7 @@ static int geneve_fill_info(struct sk_buff *skb, const struct net_device *dev)
{
struct geneve_dev *geneve = netdev_priv(dev);
struct ip_tunnel_info *info = &geneve->info;
+ bool metadata = geneve->collect_md;
__u8 tmp_vni[3];
__u32 vni;
@@ -1511,32 +1512,24 @@ 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 (!metadata && 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;
-
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 (!metadata) {
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) ||
@@ -1546,10 +1539,13 @@ static int geneve_fill_info(struct sk_buff *skb, const struct net_device *dev)
if (nla_put_be16(skb, IFLA_GENEVE_PORT, info->key.tp_dst))
goto nla_put_failure;
- if (geneve->collect_md) {
- if (nla_put_flag(skb, IFLA_GENEVE_COLLECT_METADATA))
+ if (metadata && nla_put_flag(skb, IFLA_GENEVE_COLLECT_METADATA))
goto nla_put_failure;
- }
+
+ if (nla_put_u8(skb, IFLA_GENEVE_UDP_ZERO_CSUM6_RX,
+ !geneve->use_udp6_rx_checksums))
+ goto nla_put_failure;
+
return 0;
nla_put_failure:
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCHv2 net] geneve: fix fill_info when link down
2017-11-15 1:43 ` [PATCHv2 net] geneve: fix fill_info when " Hangbin Liu
@ 2017-11-15 10:48 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2017-11-15 10:48 UTC (permalink / raw)
To: liuhangbin; +Cc: netdev, e, pshelar, jbenc
From: Hangbin Liu <liuhangbin@gmail.com>
Date: Wed, 15 Nov 2017 09:43:09 +0800
> 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 avoid passing *_REMOTE{,6} for COLLECT_METADATA since they are
> mutually exclusive, and always show UDP_ZERO_CSUM6_RX info.
>
> Fixes: 11387fe4a98 ("geneve: fix fill_info when using collect_metadata")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-15 10:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-13 9:03 [PATCH net] geneve: show remote address and checksum info even after link down Hangbin Liu
2017-11-13 20:27 ` Eric Garver
2017-11-14 2:39 ` Hangbin Liu
2017-11-14 15:48 ` Eric Garver
2017-11-15 1:43 ` [PATCHv2 net] geneve: fix fill_info when " Hangbin Liu
2017-11-15 10:48 ` David Miller
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).