diff mbox series

[jkirsher/next-queue,2/5] fm10k: Fix VLAN configuration for macvlan offload

Message ID 20171102233336.15146.31137.stgit@localhost.localdomain
State Changes Requested
Delegated to: Jeff Kirsher
Headers show
Series macvlan offload fixes | expand

Commit Message

Alexander H Duyck Nov. 2, 2017, 11:33 p.m. UTC 
From: Alexander Duyck <alexander.h.duyck@intel.com>

The fm10k driver didn't work correctly when macvlan offload was enabled.
Specifically what would occur is that we would see no unicast packets being
received. This was traced down to us not correctly configuring the default
VLAN ID for the port and defaulting to 0.

To correct this we either use the default ID provided by the switch or
simply use 1. With that we are able to pass and receive traffic without any
issues.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jesse Brandeburg Nov. 3, 2017, 5:05 p.m. UTC | #1 
On Thu, 2 Nov 2017 16:33:45 -0700
Alexander Duyck <alexander.duyck@gmail.com> wrote:

> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> The fm10k driver didn't work correctly when macvlan offload was enabled.
> Specifically what would occur is that we would see no unicast packets being
> received. This was traced down to us not correctly configuring the default
> VLAN ID for the port and defaulting to 0.
> 
> To correct this we either use the default ID provided by the switch or
> simply use 1. With that we are able to pass and receive traffic without any
> issues.

Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Keller, Jacob E Nov. 8, 2017, 10:05 p.m. UTC | #2 
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf Of
> Jesse Brandeburg
> Sent: Friday, November 03, 2017 10:06 AM
> To: Alexander Duyck <alexander.duyck@gmail.com>
> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [jkirsher/next-queue PATCH 2/5] fm10k: Fix VLAN
> configuration for macvlan offload
> 
> On Thu, 2 Nov 2017 16:33:45 -0700
> Alexander Duyck <alexander.duyck@gmail.com> wrote:
> 
> > From: Alexander Duyck <alexander.h.duyck@intel.com>
> >
> > The fm10k driver didn't work correctly when macvlan offload was enabled.
> > Specifically what would occur is that we would see no unicast packets being
> > received. This was traced down to us not correctly configuring the default
> > VLAN ID for the port and defaulting to 0.
> >
> > To correct this we either use the default ID provided by the switch or
> > simply use 1. With that we are able to pass and receive traffic without any
> > issues.
> 
> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 

Hi,

I think this isn't quite right, since we recently made changes to the fm10k code to stop assuming VLAN 1 in these cases. I believe it should just pass the default_vid

Thanks,
Jake
Alexander H Duyck Nov. 8, 2017, 11:03 p.m. UTC | #3 
On Wed, Nov 8, 2017 at 2:05 PM, Keller, Jacob E
<jacob.e.keller@intel.com> wrote:
>> -----Original Message-----
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf Of
>> Jesse Brandeburg
>> Sent: Friday, November 03, 2017 10:06 AM
>> To: Alexander Duyck <alexander.duyck@gmail.com>
>> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
>> Subject: Re: [Intel-wired-lan] [jkirsher/next-queue PATCH 2/5] fm10k: Fix VLAN
>> configuration for macvlan offload
>>
>> On Thu, 2 Nov 2017 16:33:45 -0700
>> Alexander Duyck <alexander.duyck@gmail.com> wrote:
>>
>> > From: Alexander Duyck <alexander.h.duyck@intel.com>
>> >
>> > The fm10k driver didn't work correctly when macvlan offload was enabled.
>> > Specifically what would occur is that we would see no unicast packets being
>> > received. This was traced down to us not correctly configuring the default
>> > VLAN ID for the port and defaulting to 0.
>> >
>> > To correct this we either use the default ID provided by the switch or
>> > simply use 1. With that we are able to pass and receive traffic without any
>> > issues.
>>
>> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
>>
>
> Hi,
>
> I think this isn't quite right, since we recently made changes to the fm10k code to stop assuming VLAN 1 in these cases. I believe it should just pass the default_vid
>
> Thanks,
> Jake
>

I kind of figured that might be the case. We can probably drop this
patch for now and if you want you can work this from our end as the
out-of-tree code doesn't make the upstream and I would imagine you
guys are planning to upstream that patch at some point.

- Alex
Keller, Jacob E Nov. 9, 2017, 12:21 a.m. UTC | #4 
> -----Original Message-----
> From: Alexander Duyck [mailto:alexander.duyck@gmail.com]
> Sent: Wednesday, November 08, 2017 3:04 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: Brandeburg, Jesse <jesse.brandeburg@intel.com>; netdev@vger.kernel.org;
> intel-wired-lan@lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [jkirsher/next-queue PATCH 2/5] fm10k: Fix VLAN
> configuration for macvlan offload
> 
> On Wed, Nov 8, 2017 at 2:05 PM, Keller, Jacob E
> <jacob.e.keller@intel.com> wrote:
> >> -----Original Message-----
> >> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of
> >> Jesse Brandeburg
> >> Sent: Friday, November 03, 2017 10:06 AM
> >> To: Alexander Duyck <alexander.duyck@gmail.com>
> >> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
> >> Subject: Re: [Intel-wired-lan] [jkirsher/next-queue PATCH 2/5] fm10k: Fix
> VLAN
> >> configuration for macvlan offload
> >>
> >> On Thu, 2 Nov 2017 16:33:45 -0700
> >> Alexander Duyck <alexander.duyck@gmail.com> wrote:
> >>
> >> > From: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >
> >> > The fm10k driver didn't work correctly when macvlan offload was enabled.
> >> > Specifically what would occur is that we would see no unicast packets being
> >> > received. This was traced down to us not correctly configuring the default
> >> > VLAN ID for the port and defaulting to 0.
> >> >
> >> > To correct this we either use the default ID provided by the switch or
> >> > simply use 1. With that we are able to pass and receive traffic without any
> >> > issues.
> >>
> >> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> >>
> >
> > Hi,
> >
> > I think this isn't quite right, since we recently made changes to the fm10k code
> to stop assuming VLAN 1 in these cases. I believe it should just pass the
> default_vid
> >
> > Thanks,
> > Jake
> >
> 
> I kind of figured that might be the case. We can probably drop this
> patch for now and if you want you can work this from our end as the
> out-of-tree code doesn't make the upstream and I would imagine you
> guys are planning to upstream that patch at some point.
> 
> - Alex

I think the patch should just be changed from using

hw->mac.default_vid ? : 1

to just using hw->mac.default_vid directly.

Otherwise, I think the patch is necessary.

Thanks,
Jake
Alexander H Duyck Nov. 9, 2017, 12:32 a.m. UTC | #5 
On Wed, Nov 8, 2017 at 4:21 PM, Keller, Jacob E
<jacob.e.keller@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: Alexander Duyck [mailto:alexander.duyck@gmail.com]
>> Sent: Wednesday, November 08, 2017 3:04 PM
>> To: Keller, Jacob E <jacob.e.keller@intel.com>
>> Cc: Brandeburg, Jesse <jesse.brandeburg@intel.com>; netdev@vger.kernel.org;
>> intel-wired-lan@lists.osuosl.org
>> Subject: Re: [Intel-wired-lan] [jkirsher/next-queue PATCH 2/5] fm10k: Fix VLAN
>> configuration for macvlan offload
>>
>> On Wed, Nov 8, 2017 at 2:05 PM, Keller, Jacob E
>> <jacob.e.keller@intel.com> wrote:
>> >> -----Original Message-----
>> >> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
>> Of
>> >> Jesse Brandeburg
>> >> Sent: Friday, November 03, 2017 10:06 AM
>> >> To: Alexander Duyck <alexander.duyck@gmail.com>
>> >> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
>> >> Subject: Re: [Intel-wired-lan] [jkirsher/next-queue PATCH 2/5] fm10k: Fix
>> VLAN
>> >> configuration for macvlan offload
>> >>
>> >> On Thu, 2 Nov 2017 16:33:45 -0700
>> >> Alexander Duyck <alexander.duyck@gmail.com> wrote:
>> >>
>> >> > From: Alexander Duyck <alexander.h.duyck@intel.com>
>> >> >
>> >> > The fm10k driver didn't work correctly when macvlan offload was enabled.
>> >> > Specifically what would occur is that we would see no unicast packets being
>> >> > received. This was traced down to us not correctly configuring the default
>> >> > VLAN ID for the port and defaulting to 0.
>> >> >
>> >> > To correct this we either use the default ID provided by the switch or
>> >> > simply use 1. With that we are able to pass and receive traffic without any
>> >> > issues.
>> >>
>> >> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
>> >>
>> >
>> > Hi,
>> >
>> > I think this isn't quite right, since we recently made changes to the fm10k code
>> to stop assuming VLAN 1 in these cases. I believe it should just pass the
>> default_vid
>> >
>> > Thanks,
>> > Jake
>> >
>>
>> I kind of figured that might be the case. We can probably drop this
>> patch for now and if you want you can work this from our end as the
>> out-of-tree code doesn't make the upstream and I would imagine you
>> guys are planning to upstream that patch at some point.
>>
>> - Alex
>
> I think the patch should just be changed from using
>
> hw->mac.default_vid ? : 1
>
> to just using hw->mac.default_vid directly.
>
> Otherwise, I think the patch is necessary.
>
> Thanks,
> Jake

Doesn't passing a 0 in the case of the value not being populated cause
issues? If I understand the problem I thought passing a 0 in the vlan
ID field caused some sort of error.

- Alex
Keller, Jacob E Nov. 9, 2017, 1:26 a.m. UTC | #6 
> -----Original Message-----
> From: Alexander Duyck [mailto:alexander.duyck@gmail.com]
> Sent: Wednesday, November 08, 2017 4:32 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: Brandeburg, Jesse <jesse.brandeburg@intel.com>; netdev@vger.kernel.org;
> intel-wired-lan@lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [jkirsher/next-queue PATCH 2/5] fm10k: Fix VLAN
> configuration for macvlan offload
> 
> On Wed, Nov 8, 2017 at 4:21 PM, Keller, Jacob E
> <jacob.e.keller@intel.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Alexander Duyck [mailto:alexander.duyck@gmail.com]
> >> Sent: Wednesday, November 08, 2017 3:04 PM
> >> To: Keller, Jacob E <jacob.e.keller@intel.com>
> >> Cc: Brandeburg, Jesse <jesse.brandeburg@intel.com>;
> netdev@vger.kernel.org;
> >> intel-wired-lan@lists.osuosl.org
> >> Subject: Re: [Intel-wired-lan] [jkirsher/next-queue PATCH 2/5] fm10k: Fix
> VLAN
> >> configuration for macvlan offload
> >>
> >> On Wed, Nov 8, 2017 at 2:05 PM, Keller, Jacob E
> >> <jacob.e.keller@intel.com> wrote:
> >> >> -----Original Message-----
> >> >> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf
> >> Of
> >> >> Jesse Brandeburg
> >> >> Sent: Friday, November 03, 2017 10:06 AM
> >> >> To: Alexander Duyck <alexander.duyck@gmail.com>
> >> >> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
> >> >> Subject: Re: [Intel-wired-lan] [jkirsher/next-queue PATCH 2/5] fm10k: Fix
> >> VLAN
> >> >> configuration for macvlan offload
> >> >>
> >> >> On Thu, 2 Nov 2017 16:33:45 -0700
> >> >> Alexander Duyck <alexander.duyck@gmail.com> wrote:
> >> >>
> >> >> > From: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >> >
> >> >> > The fm10k driver didn't work correctly when macvlan offload was
> enabled.
> >> >> > Specifically what would occur is that we would see no unicast packets
> being
> >> >> > received. This was traced down to us not correctly configuring the default
> >> >> > VLAN ID for the port and defaulting to 0.
> >> >> >
> >> >> > To correct this we either use the default ID provided by the switch or
> >> >> > simply use 1. With that we are able to pass and receive traffic without any
> >> >> > issues.
> >> >>
> >> >> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> >> >>
> >> >
> >> > Hi,
> >> >
> >> > I think this isn't quite right, since we recently made changes to the fm10k
> code
> >> to stop assuming VLAN 1 in these cases. I believe it should just pass the
> >> default_vid
> >> >
> >> > Thanks,
> >> > Jake
> >> >
> >>
> >> I kind of figured that might be the case. We can probably drop this
> >> patch for now and if you want you can work this from our end as the
> >> out-of-tree code doesn't make the upstream and I would imagine you
> >> guys are planning to upstream that patch at some point.
> >>
> >> - Alex
> >
> > I think the patch should just be changed from using
> >
> > hw->mac.default_vid ? : 1
> >
> > to just using hw->mac.default_vid directly.
> >
> > Otherwise, I think the patch is necessary.
> >
> > Thanks,
> > Jake
> 
> Doesn't passing a 0 in the case of the value not being populated cause
> issues? If I understand the problem I thought passing a 0 in the vlan
> ID field caused some sort of error.
> 
> - Alex

No, it just doesn't really do anything useful. But it doesn't harm anything either.

Using 0 on the switch side causes problems for the switch manager, but in this case, if we don't have a default_vid, then we aren't talking to the switch anyways.

Thanks,
Jake
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 81e4425f0529..1280127077de 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -1490,7 +1490,7 @@  static void *fm10k_dfwd_add_station(struct net_device *dev,
 		hw->mac.ops.update_xcast_mode(hw, glort,
 					      FM10K_XCAST_MODE_MULTI);
 		fm10k_queue_mac_request(interface, glort, sdev->dev_addr,
-					0, true);
+					hw->mac.default_vid ? : 1, true);
 	}
 
 	fm10k_mbx_unlock(interface);
@@ -1530,7 +1530,7 @@  static void fm10k_dfwd_del_station(struct net_device *dev, void *priv)
 		hw->mac.ops.update_xcast_mode(hw, glort,
 					      FM10K_XCAST_MODE_NONE);
 		fm10k_queue_mac_request(interface, glort, sdev->dev_addr,
-					0, false);
+					hw->mac.default_vid ? : 1, false);
 	}
 
 	fm10k_mbx_unlock(interface);