Re: [PATCH] ip6_tunnel: Correct tos value in collect_md mode
> On 14 Jun 2017, at 1:28 PM, Peter Dawsonwrote: > > On Wed, 14 Jun 2017 10:54:31 +0800 > 严海双 wrote: > > >>> Changes since v2: >>> * mask key->tos with RT_TOS() suggested by Daniel > > Can you help me understand the rationale for this change? Is there are bug > introduced by dsfield = ip6_tclass(key->label); ? > > The RT_TOS masks out 4bits of the 8bit tos field in accordance with RFC1349 > (obsoleted by RFC2474). IPv6 does not have a TOS field. So it dosen't make > sense to apply a TOS value to the outer header of an IPv6 tunnel. > > Hi, Peter Here the tos also means Traffic Class in IPv6, see the define in struct ip_tunnel_key: u8 tos;/* TOS for IPv4, TC for IPv6 */ RT_TOS mask is suggested by Daniel, please refer to the implement in vxlan or geneve code: fl6.flowlabel = ip6_make_flowinfo(RT_TOS(tos), label); Thanks.
Re: [PATCH] ip6_tunnel: Correct tos value in collect_md mode
> On 14 Jun 2017, at 1:28 PM, Peter Dawson wrote: > > On Wed, 14 Jun 2017 10:54:31 +0800 > 严海双 wrote: > > >>> Changes since v2: >>> * mask key->tos with RT_TOS() suggested by Daniel > > Can you help me understand the rationale for this change? Is there are bug > introduced by dsfield = ip6_tclass(key->label); ? > > The RT_TOS masks out 4bits of the 8bit tos field in accordance with RFC1349 > (obsoleted by RFC2474). IPv6 does not have a TOS field. So it dosen't make > sense to apply a TOS value to the outer header of an IPv6 tunnel. > > Hi, Peter Here the tos also means Traffic Class in IPv6, see the define in struct ip_tunnel_key: u8 tos;/* TOS for IPv4, TC for IPv6 */ RT_TOS mask is suggested by Daniel, please refer to the implement in vxlan or geneve code: fl6.flowlabel = ip6_make_flowinfo(RT_TOS(tos), label); Thanks.
Re: [PATCH] ip6_tunnel: Correct tos value in collect_md mode
On Wed, 14 Jun 2017 10:54:31 +0800 严海双wrote: > > Changes since v2: > > * mask key->tos with RT_TOS() suggested by Daniel Can you help me understand the rationale for this change? Is there are bug introduced by dsfield = ip6_tclass(key->label); ? The RT_TOS masks out 4bits of the 8bit tos field in accordance with RFC1349 (obsoleted by RFC2474). IPv6 does not have a TOS field. So it dosen't make sense to apply a TOS value to the outer header of an IPv6 tunnel.
Re: [PATCH] ip6_tunnel: Correct tos value in collect_md mode
On Wed, 14 Jun 2017 10:54:31 +0800 严海双 wrote: > > Changes since v2: > > * mask key->tos with RT_TOS() suggested by Daniel Can you help me understand the rationale for this change? Is there are bug introduced by dsfield = ip6_tclass(key->label); ? The RT_TOS masks out 4bits of the 8bit tos field in accordance with RFC1349 (obsoleted by RFC2474). IPv6 does not have a TOS field. So it dosen't make sense to apply a TOS value to the outer header of an IPv6 tunnel.
Re: [PATCH] ip6_tunnel: Correct tos value in collect_md mode
> On 14 Jun 2017, at 10:48 AM, Haishuang Yan >wrote: > > Same as ip_gre, geneve and vxlan, use key->tos as tos value. > > CC: Peter Dawson > Fixes: 0e9a709560db ("ip6_tunnel, ip6_gre: fix setting of DSCP on > encapsulated packets”) > Suggested-by: Daniel Borkmann > Signed-off-by: Haishuang Yan > > --- > Changes since v2: > * Add fixes information > * mask key->tos with RT_TOS() suggested by Daniel > --- > net/ipv6/ip6_tunnel.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c > index ef99d59..6400726 100644 > --- a/net/ipv6/ip6_tunnel.c > +++ b/net/ipv6/ip6_tunnel.c > @@ -1249,7 +1249,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device > *dev, __u8 dsfield, > fl6.flowi6_proto = IPPROTO_IPIP; > fl6.daddr = key->u.ipv6.dst; > fl6.flowlabel = key->label; > - dsfield = ip6_tclass(key->label); > + dsfield = RT_TOS(key->tos); > } else { > if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT)) > encap_limit = t->parms.encap_limit; > @@ -1320,7 +1320,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device > *dev, __u8 dsfield, > fl6.flowi6_proto = IPPROTO_IPV6; > fl6.daddr = key->u.ipv6.dst; > fl6.flowlabel = key->label; > - dsfield = ip6_tclass(key->label); > + dsfield = RT_TOS(key->tos); > } else { > offset = ip6_tnl_parse_tlv_enc_lim(skb, > skb_network_header(skb)); > /* ip6_tnl_parse_tlv_enc_lim() might have reallocated skb->head > */ > -- > 1.8.3.1 > > Sorry, I forgot to add subject prefix, please ignore this patch.
Re: [PATCH] ip6_tunnel: Correct tos value in collect_md mode
> On 14 Jun 2017, at 10:48 AM, Haishuang Yan > wrote: > > Same as ip_gre, geneve and vxlan, use key->tos as tos value. > > CC: Peter Dawson > Fixes: 0e9a709560db ("ip6_tunnel, ip6_gre: fix setting of DSCP on > encapsulated packets”) > Suggested-by: Daniel Borkmann > Signed-off-by: Haishuang Yan > > --- > Changes since v2: > * Add fixes information > * mask key->tos with RT_TOS() suggested by Daniel > --- > net/ipv6/ip6_tunnel.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c > index ef99d59..6400726 100644 > --- a/net/ipv6/ip6_tunnel.c > +++ b/net/ipv6/ip6_tunnel.c > @@ -1249,7 +1249,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device > *dev, __u8 dsfield, > fl6.flowi6_proto = IPPROTO_IPIP; > fl6.daddr = key->u.ipv6.dst; > fl6.flowlabel = key->label; > - dsfield = ip6_tclass(key->label); > + dsfield = RT_TOS(key->tos); > } else { > if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT)) > encap_limit = t->parms.encap_limit; > @@ -1320,7 +1320,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device > *dev, __u8 dsfield, > fl6.flowi6_proto = IPPROTO_IPV6; > fl6.daddr = key->u.ipv6.dst; > fl6.flowlabel = key->label; > - dsfield = ip6_tclass(key->label); > + dsfield = RT_TOS(key->tos); > } else { > offset = ip6_tnl_parse_tlv_enc_lim(skb, > skb_network_header(skb)); > /* ip6_tnl_parse_tlv_enc_lim() might have reallocated skb->head > */ > -- > 1.8.3.1 > > Sorry, I forgot to add subject prefix, please ignore this patch.
[PATCH] ip6_tunnel: Correct tos value in collect_md mode
Same as ip_gre, geneve and vxlan, use key->tos as tos value. CC: Peter DawsonFixes: 0e9a709560db ("ip6_tunnel, ip6_gre: fix setting of DSCP on encapsulated packets”) Suggested-by: Daniel Borkmann Signed-off-by: Haishuang Yan --- Changes since v2: * Add fixes information * mask key->tos with RT_TOS() suggested by Daniel --- net/ipv6/ip6_tunnel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c index ef99d59..6400726 100644 --- a/net/ipv6/ip6_tunnel.c +++ b/net/ipv6/ip6_tunnel.c @@ -1249,7 +1249,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPIP; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = RT_TOS(key->tos); } else { if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT)) encap_limit = t->parms.encap_limit; @@ -1320,7 +1320,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPV6; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = RT_TOS(key->tos); } else { offset = ip6_tnl_parse_tlv_enc_lim(skb, skb_network_header(skb)); /* ip6_tnl_parse_tlv_enc_lim() might have reallocated skb->head */ -- 1.8.3.1
[PATCH] ip6_tunnel: Correct tos value in collect_md mode
Same as ip_gre, geneve and vxlan, use key->tos as tos value. CC: Peter Dawson Fixes: 0e9a709560db ("ip6_tunnel, ip6_gre: fix setting of DSCP on encapsulated packets”) Suggested-by: Daniel Borkmann Signed-off-by: Haishuang Yan --- Changes since v2: * Add fixes information * mask key->tos with RT_TOS() suggested by Daniel --- net/ipv6/ip6_tunnel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c index ef99d59..6400726 100644 --- a/net/ipv6/ip6_tunnel.c +++ b/net/ipv6/ip6_tunnel.c @@ -1249,7 +1249,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPIP; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = RT_TOS(key->tos); } else { if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT)) encap_limit = t->parms.encap_limit; @@ -1320,7 +1320,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPV6; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = RT_TOS(key->tos); } else { offset = ip6_tnl_parse_tlv_enc_lim(skb, skb_network_header(skb)); /* ip6_tnl_parse_tlv_enc_lim() might have reallocated skb->head */ -- 1.8.3.1
Re: [PATCH] ip6_tunnel: Correct tos value in collect_md mode
On 06/13/2017 03:31 PM, 严海双 wrote: On 13 Jun 2017, at 5:57 PM, Daniel Borkmannwrote: On 06/13/2017 09:55 AM, Haishuang Yan wrote: Same as ip_gre, geneve and vxlan, use key->tos as tos value. Please also add Fixes tag and Cc original authors. Fixes: 0e9a709560db ("ip6_tunnel, ip6_gre: fix setting of DSCP on encapsulated packets”) Ok, I will add it, thanks Signed-off-by: Haishuang Yan --- net/ipv6/ip6_tunnel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c index ef99d59..5f4aff5 100644 --- a/net/ipv6/ip6_tunnel.c +++ b/net/ipv6/ip6_tunnel.c @@ -1249,7 +1249,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPIP; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = key->tos; Not exactly the same as vxlan / geneve. They do use key->tos indeed, but they also set fl6.flowlabel differently: [...] prio = info->key.tos; [...] fl6->flowlabel = ip6_make_flowinfo(RT_TOS(prio), info->key.label); [...] But looks like the latter is done later in ip6_tnl_xmit() eventually before the route lookup as per 5f733ee68f9a ("ip6_tunnel: fix traffic class routing for tunnels") fix. However, you still might want to mask key->tos with RT_TOS(). Do you mean this way? This one from my last comment: dsfield = RT_TOS(key->tos)
Re: [PATCH] ip6_tunnel: Correct tos value in collect_md mode
On 06/13/2017 03:31 PM, 严海双 wrote: On 13 Jun 2017, at 5:57 PM, Daniel Borkmann wrote: On 06/13/2017 09:55 AM, Haishuang Yan wrote: Same as ip_gre, geneve and vxlan, use key->tos as tos value. Please also add Fixes tag and Cc original authors. Fixes: 0e9a709560db ("ip6_tunnel, ip6_gre: fix setting of DSCP on encapsulated packets”) Ok, I will add it, thanks Signed-off-by: Haishuang Yan --- net/ipv6/ip6_tunnel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c index ef99d59..5f4aff5 100644 --- a/net/ipv6/ip6_tunnel.c +++ b/net/ipv6/ip6_tunnel.c @@ -1249,7 +1249,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPIP; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = key->tos; Not exactly the same as vxlan / geneve. They do use key->tos indeed, but they also set fl6.flowlabel differently: [...] prio = info->key.tos; [...] fl6->flowlabel = ip6_make_flowinfo(RT_TOS(prio), info->key.label); [...] But looks like the latter is done later in ip6_tnl_xmit() eventually before the route lookup as per 5f733ee68f9a ("ip6_tunnel: fix traffic class routing for tunnels") fix. However, you still might want to mask key->tos with RT_TOS(). Do you mean this way? This one from my last comment: dsfield = RT_TOS(key->tos)
Re: [PATCH] ip6_tunnel: Correct tos value in collect_md mode
On 06/13/2017 09:55 AM, Haishuang Yan wrote: Same as ip_gre, geneve and vxlan, use key->tos as tos value. Please also add Fixes tag and Cc original authors. Fixes: 0e9a709560db ("ip6_tunnel, ip6_gre: fix setting of DSCP on encapsulated packets") Signed-off-by: Haishuang Yan--- net/ipv6/ip6_tunnel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c index ef99d59..5f4aff5 100644 --- a/net/ipv6/ip6_tunnel.c +++ b/net/ipv6/ip6_tunnel.c @@ -1249,7 +1249,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPIP; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = key->tos; Not exactly the same as vxlan / geneve. They do use key->tos indeed, but they also set fl6.flowlabel differently: [...] prio = info->key.tos; [...] fl6->flowlabel = ip6_make_flowinfo(RT_TOS(prio), info->key.label); [...] But looks like the latter is done later in ip6_tnl_xmit() eventually before the route lookup as per 5f733ee68f9a ("ip6_tunnel: fix traffic class routing for tunnels") fix. However, you still might want to mask key->tos with RT_TOS(). } else { if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT)) encap_limit = t->parms.encap_limit; @@ -1320,7 +1320,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPV6; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = key->tos; } else { offset = ip6_tnl_parse_tlv_enc_lim(skb, skb_network_header(skb)); /* ip6_tnl_parse_tlv_enc_lim() might have reallocated skb->head */
Re: [PATCH] ip6_tunnel: Correct tos value in collect_md mode
On 06/13/2017 09:55 AM, Haishuang Yan wrote: Same as ip_gre, geneve and vxlan, use key->tos as tos value. Please also add Fixes tag and Cc original authors. Fixes: 0e9a709560db ("ip6_tunnel, ip6_gre: fix setting of DSCP on encapsulated packets") Signed-off-by: Haishuang Yan --- net/ipv6/ip6_tunnel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c index ef99d59..5f4aff5 100644 --- a/net/ipv6/ip6_tunnel.c +++ b/net/ipv6/ip6_tunnel.c @@ -1249,7 +1249,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPIP; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = key->tos; Not exactly the same as vxlan / geneve. They do use key->tos indeed, but they also set fl6.flowlabel differently: [...] prio = info->key.tos; [...] fl6->flowlabel = ip6_make_flowinfo(RT_TOS(prio), info->key.label); [...] But looks like the latter is done later in ip6_tnl_xmit() eventually before the route lookup as per 5f733ee68f9a ("ip6_tunnel: fix traffic class routing for tunnels") fix. However, you still might want to mask key->tos with RT_TOS(). } else { if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT)) encap_limit = t->parms.encap_limit; @@ -1320,7 +1320,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPV6; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = key->tos; } else { offset = ip6_tnl_parse_tlv_enc_lim(skb, skb_network_header(skb)); /* ip6_tnl_parse_tlv_enc_lim() might have reallocated skb->head */
[PATCH] ip6_tunnel: Correct tos value in collect_md mode
Same as ip_gre, geneve and vxlan, use key->tos as tos value. Signed-off-by: Haishuang Yan--- net/ipv6/ip6_tunnel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c index ef99d59..5f4aff5 100644 --- a/net/ipv6/ip6_tunnel.c +++ b/net/ipv6/ip6_tunnel.c @@ -1249,7 +1249,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPIP; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = key->tos; } else { if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT)) encap_limit = t->parms.encap_limit; @@ -1320,7 +1320,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPV6; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = key->tos; } else { offset = ip6_tnl_parse_tlv_enc_lim(skb, skb_network_header(skb)); /* ip6_tnl_parse_tlv_enc_lim() might have reallocated skb->head */ -- 1.8.3.1
[PATCH] ip6_tunnel: Correct tos value in collect_md mode
Same as ip_gre, geneve and vxlan, use key->tos as tos value. Signed-off-by: Haishuang Yan --- net/ipv6/ip6_tunnel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c index ef99d59..5f4aff5 100644 --- a/net/ipv6/ip6_tunnel.c +++ b/net/ipv6/ip6_tunnel.c @@ -1249,7 +1249,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPIP; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = key->tos; } else { if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT)) encap_limit = t->parms.encap_limit; @@ -1320,7 +1320,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, fl6.flowi6_proto = IPPROTO_IPV6; fl6.daddr = key->u.ipv6.dst; fl6.flowlabel = key->label; - dsfield = ip6_tclass(key->label); + dsfield = key->tos; } else { offset = ip6_tnl_parse_tlv_enc_lim(skb, skb_network_header(skb)); /* ip6_tnl_parse_tlv_enc_lim() might have reallocated skb->head */ -- 1.8.3.1