| Message ID | 20190626080844.20796-1-yuehaibing@huawei.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | bonding: Always enable vlan tx offload | expand |
Wed, Jun 26, 2019 at 10:08:44AM CEST, yuehaibing@huawei.com wrote: >We build vlan on top of bonding interface, which vlan offload >is off, bond mode is 802.3ad (LACP) and xmit_hash_policy is >BOND_XMIT_POLICY_ENCAP34. > >Because vlan tx offload is off, vlan tci is cleared and skb push >the vlan header in validate_xmit_vlan() while sending from vlan >devices. Then in bond_xmit_hash, __skb_flow_dissect() fails to >get information from protocol headers encapsulated within vlan, >because 'nhoff' is points to IP header, so bond hashing is based >on layer 2 info, which fails to distribute packets across slaves. > >This patch always enable bonding's vlan tx offload, pass the vlan >packets to the slave devices with vlan tci, let them to handle >vlan implementation. > >Fixes: 278339a42a1b ("bonding: propogate vlan_features to bonding master") >Suggested-by: Jiri Pirko <jiri@resnulli.us> >Signed-off-by: YueHaibing <yuehaibing@huawei.com> Acked-by: Jiri Pirko <jiri@mellanox.com> Could you please do the same for team? Thanks!
On 2019/6/26 23:25, Jiri Pirko wrote: > Wed, Jun 26, 2019 at 10:08:44AM CEST, yuehaibing@huawei.com wrote: >> We build vlan on top of bonding interface, which vlan offload >> is off, bond mode is 802.3ad (LACP) and xmit_hash_policy is >> BOND_XMIT_POLICY_ENCAP34. >> >> Because vlan tx offload is off, vlan tci is cleared and skb push >> the vlan header in validate_xmit_vlan() while sending from vlan >> devices. Then in bond_xmit_hash, __skb_flow_dissect() fails to >> get information from protocol headers encapsulated within vlan, >> because 'nhoff' is points to IP header, so bond hashing is based >> on layer 2 info, which fails to distribute packets across slaves. >> >> This patch always enable bonding's vlan tx offload, pass the vlan >> packets to the slave devices with vlan tci, let them to handle >> vlan implementation. >> >> Fixes: 278339a42a1b ("bonding: propogate vlan_features to bonding master") >> Suggested-by: Jiri Pirko <jiri@resnulli.us> >> Signed-off-by: YueHaibing <yuehaibing@huawei.com> > > Acked-by: Jiri Pirko <jiri@mellanox.com> > > Could you please do the same for team? Thanks! Sure, will send it, thank you! > > . >
On Wed, Jun 26, 2019 at 04:08:44PM +0800, YueHaibing wrote: > We build vlan on top of bonding interface, which vlan offload > is off, bond mode is 802.3ad (LACP) and xmit_hash_policy is > BOND_XMIT_POLICY_ENCAP34. > > Because vlan tx offload is off, vlan tci is cleared and skb push > the vlan header in validate_xmit_vlan() while sending from vlan > devices. Then in bond_xmit_hash, __skb_flow_dissect() fails to > get information from protocol headers encapsulated within vlan, > because 'nhoff' is points to IP header, so bond hashing is based > on layer 2 info, which fails to distribute packets across slaves. > > This patch always enable bonding's vlan tx offload, pass the vlan > packets to the slave devices with vlan tci, let them to handle > vlan implementation. [...] > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > index 407f4095a37a..799fc38c5c34 100644 > --- a/drivers/net/bonding/bond_main.c > +++ b/drivers/net/bonding/bond_main.c > @@ -4320,12 +4320,12 @@ void bond_setup(struct net_device *bond_dev) > bond_dev->features |= NETIF_F_NETNS_LOCAL; > > bond_dev->hw_features = BOND_VLAN_FEATURES | > - NETIF_F_HW_VLAN_CTAG_TX | > NETIF_F_HW_VLAN_CTAG_RX | > NETIF_F_HW_VLAN_CTAG_FILTER; > > bond_dev->hw_features |= NETIF_F_GSO_ENCAP_ALL | NETIF_F_GSO_UDP_L4; > bond_dev->features |= bond_dev->hw_features; > + bond_dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX; > } > > /* Destroy a bonding device. > I can see that bonding driver uses dev_queue_xmit() to pass packets to slave links, but I can't see where in the path it does software fallback for devices without HW VLAN tagging. Generally drivers that don't ever do VLAN offload also ignore vlan_tci presence. Am I missing something here? Best Regards, Michał Mirosław
Wed, Jun 26, 2019 at 06:13:38PM CEST, mirq-linux@rere.qmqm.pl wrote: >On Wed, Jun 26, 2019 at 04:08:44PM +0800, YueHaibing wrote: >> We build vlan on top of bonding interface, which vlan offload >> is off, bond mode is 802.3ad (LACP) and xmit_hash_policy is >> BOND_XMIT_POLICY_ENCAP34. >> >> Because vlan tx offload is off, vlan tci is cleared and skb push >> the vlan header in validate_xmit_vlan() while sending from vlan >> devices. Then in bond_xmit_hash, __skb_flow_dissect() fails to >> get information from protocol headers encapsulated within vlan, >> because 'nhoff' is points to IP header, so bond hashing is based >> on layer 2 info, which fails to distribute packets across slaves. >> >> This patch always enable bonding's vlan tx offload, pass the vlan >> packets to the slave devices with vlan tci, let them to handle >> vlan implementation. >[...] >> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >> index 407f4095a37a..799fc38c5c34 100644 >> --- a/drivers/net/bonding/bond_main.c >> +++ b/drivers/net/bonding/bond_main.c >> @@ -4320,12 +4320,12 @@ void bond_setup(struct net_device *bond_dev) >> bond_dev->features |= NETIF_F_NETNS_LOCAL; >> >> bond_dev->hw_features = BOND_VLAN_FEATURES | >> - NETIF_F_HW_VLAN_CTAG_TX | >> NETIF_F_HW_VLAN_CTAG_RX | >> NETIF_F_HW_VLAN_CTAG_FILTER; >> >> bond_dev->hw_features |= NETIF_F_GSO_ENCAP_ALL | NETIF_F_GSO_UDP_L4; >> bond_dev->features |= bond_dev->hw_features; >> + bond_dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX; >> } >> >> /* Destroy a bonding device. >> > >I can see that bonding driver uses dev_queue_xmit() to pass packets to >slave links, but I can't see where in the path it does software fallback >for devices without HW VLAN tagging. Generally drivers that don't ever >do VLAN offload also ignore vlan_tci presence. Am I missing something >here? validate_xmit_skb->validate_xmit_vlan > >Best Regards, >Michał Mirosław
On Wed, Jun 26, 2019 at 06:48:53PM +0200, Jiri Pirko wrote: > Wed, Jun 26, 2019 at 06:13:38PM CEST, mirq-linux@rere.qmqm.pl wrote: > >On Wed, Jun 26, 2019 at 04:08:44PM +0800, YueHaibing wrote: [...] > >> This patch always enable bonding's vlan tx offload, pass the vlan > >> packets to the slave devices with vlan tci, let them to handle > >> vlan implementation. [...] > >I can see that bonding driver uses dev_queue_xmit() to pass packets to > >slave links, but I can't see where in the path it does software fallback > >for devices without HW VLAN tagging. Generally drivers that don't ever > >do VLAN offload also ignore vlan_tci presence. Am I missing something > >here? > validate_xmit_skb->validate_xmit_vlan Yes it is. Thanks! Best Regards, Michał Mirosław
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 407f4095a37a..799fc38c5c34 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -4320,12 +4320,12 @@ void bond_setup(struct net_device *bond_dev) bond_dev->features |= NETIF_F_NETNS_LOCAL; bond_dev->hw_features = BOND_VLAN_FEATURES | - NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_CTAG_FILTER; bond_dev->hw_features |= NETIF_F_GSO_ENCAP_ALL | NETIF_F_GSO_UDP_L4; bond_dev->features |= bond_dev->hw_features; + bond_dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX; } /* Destroy a bonding device.
We build vlan on top of bonding interface, which vlan offload is off, bond mode is 802.3ad (LACP) and xmit_hash_policy is BOND_XMIT_POLICY_ENCAP34. Because vlan tx offload is off, vlan tci is cleared and skb push the vlan header in validate_xmit_vlan() while sending from vlan devices. Then in bond_xmit_hash, __skb_flow_dissect() fails to get information from protocol headers encapsulated within vlan, because 'nhoff' is points to IP header, so bond hashing is based on layer 2 info, which fails to distribute packets across slaves. This patch always enable bonding's vlan tx offload, pass the vlan packets to the slave devices with vlan tci, let them to handle vlan implementation. Fixes: 278339a42a1b ("bonding: propogate vlan_features to bonding master") Suggested-by: Jiri Pirko <jiri@resnulli.us> Signed-off-by: YueHaibing <yuehaibing@huawei.com> --- drivers/net/bonding/bond_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)