| Message ID | 1594180136-15912-3-git-send-email-tanhuazhong@huawei.com |
|---|---|
| State | RFC |
| Delegated to: | David Miller |
| Headers | show |
| Series | [RFC,net-next,1/2] udp: add NETIF_F_GSO_UDP_L4 to NETIF_F_SOFTWARE_GSO | expand |
On 7/7/20 8:48 PM, Huazhong Tan wrote: > Since UDP GSO feature is depended on checksum offload, so disable > UDP GSO feature when CSUM is disabled, then from user-space also > can see UDP GSO feature is disabled. > > Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com> > --- > net/core/dev.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/net/core/dev.c b/net/core/dev.c > index c02bae9..dcb6b35 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -9095,6 +9095,12 @@ static netdev_features_t netdev_fix_features(struct net_device *dev, > features &= ~NETIF_F_TSO6; > } > > + if ((features & NETIF_F_GSO_UDP_L4) && !(features & NETIF_F_HW_CSUM) && > + (!(features & NETIF_F_IP_CSUM) || !(features & NETIF_F_IPV6_CSUM))) { This would prevent a device providing IPv4 checksum only (no IPv6 csum support) from sending IPv4 UDP GSO packets ? > + netdev_dbg(dev, "Dropping UDP GSO features since no CSUM feature.\n"); > + features &= ~NETIF_F_GSO_UDP_L4; > + } > + > /* TSO with IPv4 ID mangling requires IPv4 TSO be enabled */ > if ((features & NETIF_F_TSO_MANGLEID) && !(features & NETIF_F_TSO)) > features &= ~NETIF_F_TSO_MANGLEID; >
On 2020/7/8 13:36, Eric Dumazet wrote: > > > On 7/7/20 8:48 PM, Huazhong Tan wrote: >> Since UDP GSO feature is depended on checksum offload, so disable >> UDP GSO feature when CSUM is disabled, then from user-space also >> can see UDP GSO feature is disabled. >> >> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com> >> --- >> net/core/dev.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/net/core/dev.c b/net/core/dev.c >> index c02bae9..dcb6b35 100644 >> --- a/net/core/dev.c >> +++ b/net/core/dev.c >> @@ -9095,6 +9095,12 @@ static netdev_features_t netdev_fix_features(struct net_device *dev, >> features &= ~NETIF_F_TSO6; >> } >> >> + if ((features & NETIF_F_GSO_UDP_L4) && !(features & NETIF_F_HW_CSUM) && >> + (!(features & NETIF_F_IP_CSUM) || !(features & NETIF_F_IPV6_CSUM))) { > > This would prevent a device providing IPv4 checksum only (no IPv6 csum support) from sending IPv4 UDP GSO packets ? > Yes, not like TCP (who uses NETIF_F_TSO for IPv4 and NETIF_F_TSO6 for IPv6), UDP only has a NETIF_F_GSO_UDP_L4 for both IPv4 and IPv6. I cannot find a better way to do it with combined IPv4 and IPv6 csum together. For this issue, is there any good idea to fix it? >> + netdev_dbg(dev, "Dropping UDP GSO features since no CSUM feature.\n"); >> + features &= ~NETIF_F_GSO_UDP_L4; >> + } >> + >> /* TSO with IPv4 ID mangling requires IPv4 TSO be enabled */ >> if ((features & NETIF_F_TSO_MANGLEID) && !(features & NETIF_F_TSO)) >> features &= ~NETIF_F_TSO_MANGLEID; >> > >
On 7/8/20 7:30 PM, tanhuazhong wrote: > > > On 2020/7/8 13:36, Eric Dumazet wrote: >> >> >> On 7/7/20 8:48 PM, Huazhong Tan wrote: >>> Since UDP GSO feature is depended on checksum offload, so disable >>> UDP GSO feature when CSUM is disabled, then from user-space also >>> can see UDP GSO feature is disabled. >>> >>> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com> >>> --- >>> net/core/dev.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/net/core/dev.c b/net/core/dev.c >>> index c02bae9..dcb6b35 100644 >>> --- a/net/core/dev.c >>> +++ b/net/core/dev.c >>> @@ -9095,6 +9095,12 @@ static netdev_features_t netdev_fix_features(struct net_device *dev, >>> features &= ~NETIF_F_TSO6; >>> } >>> + if ((features & NETIF_F_GSO_UDP_L4) && !(features & NETIF_F_HW_CSUM) && >>> + (!(features & NETIF_F_IP_CSUM) || !(features & NETIF_F_IPV6_CSUM))) { >> >> This would prevent a device providing IPv4 checksum only (no IPv6 csum support) from sending IPv4 UDP GSO packets ? >> > > Yes, not like TCP (who uses NETIF_F_TSO for IPv4 and NETIF_F_TSO6 for IPv6), > UDP only has a NETIF_F_GSO_UDP_L4 for both IPv4 and IPv6. > I cannot find a better way to do it with combined IPv4 and IPv6 csum together. > For this issue, is there any good idea to fix it? This could be done in an ndo_fix_features(), or ndo_features_check() Or maybe we do not care, but this should probably be documented.
On 2020/7/9 10:47, Eric Dumazet wrote: > > > On 7/8/20 7:30 PM, tanhuazhong wrote: >> >> >> On 2020/7/8 13:36, Eric Dumazet wrote: >>> >>> >>> On 7/7/20 8:48 PM, Huazhong Tan wrote: >>>> Since UDP GSO feature is depended on checksum offload, so disable >>>> UDP GSO feature when CSUM is disabled, then from user-space also >>>> can see UDP GSO feature is disabled. >>>> >>>> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com> >>>> --- >>>> net/core/dev.c | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>>> diff --git a/net/core/dev.c b/net/core/dev.c >>>> index c02bae9..dcb6b35 100644 >>>> --- a/net/core/dev.c >>>> +++ b/net/core/dev.c >>>> @@ -9095,6 +9095,12 @@ static netdev_features_t netdev_fix_features(struct net_device *dev, >>>> features &= ~NETIF_F_TSO6; >>>> } >>>> + if ((features & NETIF_F_GSO_UDP_L4) && !(features & NETIF_F_HW_CSUM) && >>>> + (!(features & NETIF_F_IP_CSUM) || !(features & NETIF_F_IPV6_CSUM))) { >>> >>> This would prevent a device providing IPv4 checksum only (no IPv6 csum support) from sending IPv4 UDP GSO packets ? >>> >> >> Yes, not like TCP (who uses NETIF_F_TSO for IPv4 and NETIF_F_TSO6 for IPv6), >> UDP only has a NETIF_F_GSO_UDP_L4 for both IPv4 and IPv6. >> I cannot find a better way to do it with combined IPv4 and IPv6 csum together. >> For this issue, is there any good idea to fix it? > > This could be done in an ndo_fix_features(), or ndo_features_check() > > Or maybe we do not care, but this should probably be documented. > Thanks for your suggestion. If only check NETIF_F_HW_CSUM here is more acceptable? > >
diff --git a/net/core/dev.c b/net/core/dev.c index c02bae9..dcb6b35 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -9095,6 +9095,12 @@ static netdev_features_t netdev_fix_features(struct net_device *dev, features &= ~NETIF_F_TSO6; } + if ((features & NETIF_F_GSO_UDP_L4) && !(features & NETIF_F_HW_CSUM) && + (!(features & NETIF_F_IP_CSUM) || !(features & NETIF_F_IPV6_CSUM))) { + netdev_dbg(dev, "Dropping UDP GSO features since no CSUM feature.\n"); + features &= ~NETIF_F_GSO_UDP_L4; + } + /* TSO with IPv4 ID mangling requires IPv4 TSO be enabled */ if ((features & NETIF_F_TSO_MANGLEID) && !(features & NETIF_F_TSO)) features &= ~NETIF_F_TSO_MANGLEID;
Since UDP GSO feature is depended on checksum offload, so disable UDP GSO feature when CSUM is disabled, then from user-space also can see UDP GSO feature is disabled. Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com> --- net/core/dev.c | 6 ++++++ 1 file changed, 6 insertions(+)