linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] bonding: Add vlan tx offload to hw_enc_features
@ 2019-08-05 13:49 YueHaibing
  2019-08-06 13:41 ` Jay Vosburgh
  0 siblings, 1 reply; 3+ messages in thread
From: YueHaibing @ 2019-08-05 13:49 UTC (permalink / raw)
  To: j.vosburgh, vfalico, andy, davem, jiri; +Cc: linux-kernel, netdev, YueHaibing

As commit 30d8177e8ac7 ("bonding: Always enable vlan tx offload")
said, we should always enable bonding's vlan tx offload, pass the
vlan packets to the slave devices with vlan tci, let them to handle
vlan implementation.

Now if encapsulation protocols like VXLAN is used, skb->encapsulation
may be set, then the packet is passed to vlan devicec which based on
bonding device. However in netif_skb_features(), the check of
hw_enc_features:

	 if (skb->encapsulation)
                 features &= dev->hw_enc_features;

clears NETIF_F_HW_VLAN_CTAG_TX/NETIF_F_HW_VLAN_STAG_TX. This results
in same issue in commit 30d8177e8ac7 like this:

vlan_dev_hard_start_xmit
  -->dev_queue_xmit
    -->validate_xmit_skb
      -->netif_skb_features //NETIF_F_HW_VLAN_CTAG_TX is cleared
      -->validate_xmit_vlan
        -->__vlan_hwaccel_push_inside //skb->tci is cleared
...
 --> bond_start_xmit
   --> bond_xmit_hash //BOND_XMIT_POLICY_ENCAP34
     --> __skb_flow_dissect // nhoff point to IP header
        -->  case htons(ETH_P_8021Q)
             // skb_vlan_tag_present is false, so
             vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan),
             //vlan point to ip header wrongly

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/bonding/bond_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 02fd782..931d9d9 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1126,6 +1126,8 @@ static void bond_compute_features(struct bonding *bond)
 done:
 	bond_dev->vlan_features = vlan_features;
 	bond_dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL |
+				    NETIF_F_HW_VLAN_CTAG_TX |
+				    NETIF_F_HW_VLAN_STAG_TX |
 				    NETIF_F_GSO_UDP_L4;
 	bond_dev->mpls_features = mpls_features;
 	bond_dev->gso_max_segs = gso_max_segs;
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] bonding: Add vlan tx offload to hw_enc_features
  2019-08-05 13:49 [PATCH] bonding: Add vlan tx offload to hw_enc_features YueHaibing
@ 2019-08-06 13:41 ` Jay Vosburgh
  2019-08-07  1:46   ` Yuehaibing
  0 siblings, 1 reply; 3+ messages in thread
From: Jay Vosburgh @ 2019-08-06 13:41 UTC (permalink / raw)
  To: YueHaibing; +Cc: vfalico, andy, davem, jiri, linux-kernel, netdev

YueHaibing <yuehaibing@huawei.com> wrote:

>As commit 30d8177e8ac7 ("bonding: Always enable vlan tx offload")
>said, we should always enable bonding's vlan tx offload, pass the
>vlan packets to the slave devices with vlan tci, let them to handle
>vlan implementation.
>
>Now if encapsulation protocols like VXLAN is used, skb->encapsulation
>may be set, then the packet is passed to vlan devicec which based on

	Typo: "devicec"

>bonding device. However in netif_skb_features(), the check of
>hw_enc_features:
>
>	 if (skb->encapsulation)
>                 features &= dev->hw_enc_features;
>
>clears NETIF_F_HW_VLAN_CTAG_TX/NETIF_F_HW_VLAN_STAG_TX. This results
>in same issue in commit 30d8177e8ac7 like this:
>
>vlan_dev_hard_start_xmit
>  -->dev_queue_xmit
>    -->validate_xmit_skb
>      -->netif_skb_features //NETIF_F_HW_VLAN_CTAG_TX is cleared
>      -->validate_xmit_vlan
>        -->__vlan_hwaccel_push_inside //skb->tci is cleared
>...
> --> bond_start_xmit
>   --> bond_xmit_hash //BOND_XMIT_POLICY_ENCAP34
>     --> __skb_flow_dissect // nhoff point to IP header
>        -->  case htons(ETH_P_8021Q)
>             // skb_vlan_tag_present is false, so
>             vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan),
>             //vlan point to ip header wrongly
>
>Signed-off-by: YueHaibing <yuehaibing@huawei.com>

	Looks good to me; should this be tagged with

Fixes: 278339a42a1b ("bonding: propogate vlan_features to bonding master")

	as 30d8177e8ac7 was?  If not, is there an appropriate commit id?

Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>

	-J

>---
> drivers/net/bonding/bond_main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 02fd782..931d9d9 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -1126,6 +1126,8 @@ static void bond_compute_features(struct bonding *bond)
> done:
> 	bond_dev->vlan_features = vlan_features;
> 	bond_dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL |
>+				    NETIF_F_HW_VLAN_CTAG_TX |
>+				    NETIF_F_HW_VLAN_STAG_TX |
> 				    NETIF_F_GSO_UDP_L4;
> 	bond_dev->mpls_features = mpls_features;
> 	bond_dev->gso_max_segs = gso_max_segs;
>-- 
>2.7.4

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] bonding: Add vlan tx offload to hw_enc_features
  2019-08-06 13:41 ` Jay Vosburgh
@ 2019-08-07  1:46   ` Yuehaibing
  0 siblings, 0 replies; 3+ messages in thread
From: Yuehaibing @ 2019-08-07  1:46 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: vfalico, andy, davem, jiri, linux-kernel, netdev

On 2019/8/6 21:41, Jay Vosburgh wrote:
> YueHaibing <yuehaibing@huawei.com> wrote:
> 
>> As commit 30d8177e8ac7 ("bonding: Always enable vlan tx offload")
>> said, we should always enable bonding's vlan tx offload, pass the
>> vlan packets to the slave devices with vlan tci, let them to handle
>> vlan implementation.
>>
>> Now if encapsulation protocols like VXLAN is used, skb->encapsulation
>> may be set, then the packet is passed to vlan devicec which based on
> 
> 	Typo: "devicec"

oh, yes, thanks!

> 
>> bonding device. However in netif_skb_features(), the check of
>> hw_enc_features:
>>
>> 	 if (skb->encapsulation)
>>                 features &= dev->hw_enc_features;
>>
>> clears NETIF_F_HW_VLAN_CTAG_TX/NETIF_F_HW_VLAN_STAG_TX. This results
>> in same issue in commit 30d8177e8ac7 like this:
>>
>> vlan_dev_hard_start_xmit
>>  -->dev_queue_xmit
>>    -->validate_xmit_skb
>>      -->netif_skb_features //NETIF_F_HW_VLAN_CTAG_TX is cleared
>>      -->validate_xmit_vlan
>>        -->__vlan_hwaccel_push_inside //skb->tci is cleared
>> ...
>> --> bond_start_xmit
>>   --> bond_xmit_hash //BOND_XMIT_POLICY_ENCAP34
>>     --> __skb_flow_dissect // nhoff point to IP header
>>        -->  case htons(ETH_P_8021Q)
>>             // skb_vlan_tag_present is false, so
>>             vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan),
>>             //vlan point to ip header wrongly
>>
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> 
> 	Looks good to me; should this be tagged with
> 
> Fixes: 278339a42a1b ("bonding: propogate vlan_features to bonding master")
> 
> 	as 30d8177e8ac7 was?  If not, is there an appropriate commit id?

It seems the commit was:

Fixes: b2a103e6d0af ("bonding: convert to ndo_fix_features")

> 
> Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>


Thanks, will send v2.

> 
> 	-J
> 
>> ---
>> drivers/net/bonding/bond_main.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 02fd782..931d9d9 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -1126,6 +1126,8 @@ static void bond_compute_features(struct bonding *bond)
>> done:
>> 	bond_dev->vlan_features = vlan_features;
>> 	bond_dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL |
>> +				    NETIF_F_HW_VLAN_CTAG_TX |
>> +				    NETIF_F_HW_VLAN_STAG_TX |
>> 				    NETIF_F_GSO_UDP_L4;
>> 	bond_dev->mpls_features = mpls_features;
>> 	bond_dev->gso_max_segs = gso_max_segs;
>> -- 
>> 2.7.4
> 
> ---
> 	-Jay Vosburgh, jay.vosburgh@canonical.com
> 
> .
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-08-07  1:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-05 13:49 [PATCH] bonding: Add vlan tx offload to hw_enc_features YueHaibing
2019-08-06 13:41 ` Jay Vosburgh
2019-08-07  1:46   ` Yuehaibing

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).