| Message ID | 1365778164-16857-1-git-send-email-ivecera@redhat.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Ivan Vecera <ivecera@redhat.com> Date: Fri, 12 Apr 2013 16:49:24 +0200 > The driver should use return value of __vlan_put_tag with appropriate > NULL-check instead of old skb pointer. > > Signed-off-by: Ivan Vecera <ivecera@redhat.com> I'll apply this patch but there is some strangeness with the logic that controls these code paths. The call to be_insert_vlan_in_pkt() is guarded by: be_vlan_tag_chk(adapter, skb) which evaluates to: vlan_tx_tag_present(skb) || adapter->pvid But the __vlan_put_tag() call is guarded by only one of those two conditions: vlan_tx_tag_present(skb) One of these two is wrong and should be corrected. I suspect that we have several layers of bugs here, in that we need to do __vlan_put_tag() in the adapter->pvid case but that means that tag determination needs to have some adapter->pvid logic added to it. Or, if for some reason the adapter->pvid case doesn't apply to this HW bug, the test guarding the be_insert_vlan_in_pkt() call should be changed to vlan_tx_tag_present() and a big comment added explaining why adapter->pvid is not being considered. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: David Miller [mailto:davem@davemloft.net] > > From: Ivan Vecera <ivecera@redhat.com> > > I'll apply this patch but there is some strangeness with the logic that controls > these code paths. > > The call to be_insert_vlan_in_pkt() is guarded by: > > be_vlan_tag_chk(adapter, skb) > > which evaluates to: > > vlan_tx_tag_present(skb) || adapter->pvid > > But the __vlan_put_tag() call is guarded by only one of those two > conditions: > > vlan_tx_tag_present(skb) > > One of these two is wrong and should be corrected. David, yes the pvid check here is wrong. The pvid insertion by the driver (to hack around some HW bugs) requires some support from the FW and that requires some more code/logic. That fix seems to be missing in the netdev tree. We'll send out a patch-set to handle these issues. Thanks! -Sathya -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c index 4d6f3c5..a4e4626 100644 --- a/drivers/net/ethernet/emulex/benet/be_main.c +++ b/drivers/net/ethernet/emulex/benet/be_main.c @@ -759,8 +759,9 @@ static struct sk_buff *be_insert_vlan_in_pkt(struct be_adapter *adapter, if (vlan_tx_tag_present(skb)) { vlan_tag = be_get_tx_vlan_tag(adapter, skb); - __vlan_put_tag(skb, vlan_tag); - skb->vlan_tci = 0; + skb = __vlan_put_tag(skb, vlan_tag); + if (skb) + skb->vlan_tci = 0; } return skb;
The driver should use return value of __vlan_put_tag with appropriate NULL-check instead of old skb pointer. Signed-off-by: Ivan Vecera <ivecera@redhat.com> --- drivers/net/ethernet/emulex/benet/be_main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)