RE: [PATCH] fcoe: Fix a vlan tagging problem

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Neil Horman [mailto:nhorman@xxxxxxxxxxxxx]
> Sent: Tuesday, June 25, 2013 12:26 PM
> To: Love, Robert W
> Cc: fcoe-devel@xxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] fcoe: Fix a vlan tagging problem
> 
> On Tue, Jun 25, 2013 at 07:13:51PM +0000, Love, Robert W wrote:
> > > -----Original Message-----
> > > From: Neil Horman [mailto:nhorman@xxxxxxxxxxxxx]
> > > Sent: Tuesday, June 25, 2013 12:09 PM
> > > To: fcoe-devel@xxxxxxxxxxxxx
> > > Cc: Neil Horman; Love, Robert W; stable@xxxxxxxxxxxxxxx
> > > Subject: [PATCH] fcoe: Fix a vlan tagging problem
> > >
> > > I was testing upstream FCoE and noticed that I was no longer able to
> > > find any luns, nor were any of my fc_host devices comming out of
> > > Offline state.  I took some tcpdumps of the problem and noted that
> > > my FLOGI requests were getting accepted but what should have been my
> > > PLOGI requests, were getting interpreted by wireshark as
> > > uncategorized FCP data.  There was a vlan header, but the ethertype
> > > was all zeros, causing it, and all subsequent data to get
> > > misinterpreted.  Looking further, I noted that in fcoe_xmit, the
> > > path that handles PLOGI requests does some manual vlan tag addition,
> > > rather than using vlan_put_tag as it should.  I'm honestly not sure
> > > how it ever worked, as I don't see that the manual tag addition ever
> > > set the h_vlan_proto field to
> > > htons(ETH_P_8021Q) as it should have.  I assume that just worked by
> > > luck, and some recent change in the vlan code caused it to stop
> > > functioning.  At any rate, I've tested out the below fix, and this
> > > properly tags frames with a vlan in both the non-accelerated and hw
> accelerated case.
> > >
> > > Signed-off-by: Neil Horman <nhorman@xxxxxxxxxxxxx>
> > > CC: Robert Love <robert.w.love@xxxxxxxxx>
> > > CC: stable@xxxxxxxxxxxxxxx
> > > ---
> > >  drivers/scsi/fcoe/fcoe.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> > > index
> > > fc7bb1f..731c471 100644
> > > --- a/drivers/scsi/fcoe/fcoe.c
> > > +++ b/drivers/scsi/fcoe/fcoe.c
> > > @@ -1660,9 +1660,8 @@ static int fcoe_xmit(struct fc_lport *lport,
> > > struct fc_frame *fp)
> > >
> > >  	if (fcoe->netdev->priv_flags & IFF_802_1Q_VLAN &&
> > >  	    fcoe->realdev->features & NETIF_F_HW_VLAN_CTAG_TX) {
> > > -		skb->vlan_tci = VLAN_TAG_PRESENT |
> > > -				vlan_dev_vlan_id(fcoe->netdev);
> > >  		skb->dev = fcoe->realdev;
> > > +		vlan_put_tag(skb, htons(ETH_P_8021Q),
> > > +vlan_dev_vlan_id(fcoe->netdev));
> > >  	} else
> > >  		skb->dev = fcoe->netdev;
> > >
> > > --
> > > 1.8.1.4
> >
> > Hey Neil, this is the same change as recently discussed on list here:
> >
> > http://patchwork.open-fcoe.org/patch/65/
> >
> > The only difference between yours and my patch is that I check the return
> value from vlan_put_tag.
> >
> > I'm going to send my patch up today, unless you have an objection.
> >
> ACK, works for me, please send it to stable as well if you could.
> 

OK, I missed one of John's comments, so I just sent an updated version.... I'm testing it now, but I'm going to be delayed by an upcoming block of meetings.

This problem was introduced in the 3.9 merge window. I don't think it needs to go to stable unless it misses 3.9-rc, which I think is possible. I'm on top of it.

//Rob
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]