Message ID | 20171102233336.15146.31137.stgit@localhost.localdomain |
---|---|
State | Changes Requested |
Delegated to: | Jeff Kirsher |
Headers | show |
Series | macvlan offload fixes | expand |
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>
> -----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
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
> -----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
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
> -----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 --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);