[Intel-wired-lan] [PATCH v4 1/6] i40e: Remove CONFIG_I40E_VXLAN

Tue Nov 10 19:23:24 UTC 2015

Ok. That should work, but a quick check would be helpful.

Regards,
Jake

On Tue, 2015-11-10 at 19:08 +0000, Singhai, Anjali wrote:
> Jake this patch doesn't change the use of CONFIG_VXLAN in our driver
> to guard against a potential issue that you pointed out.
> Our driver had already changed to using CONFIG_VXLAN in a previous
> patch.
> 
> But I believe that we will still be okay in the scenario that VXLAN
> module is not loaded because of two reasons
> 1) We check a flag in sync_vsi_filters if a filter was added or
> deleted before proceeding to do anything and that will return very
> quickly from the function when VXLAN or GENEVE module is not loaded.
> 2) udp_offload_get_port function has empty definitions in the base
> kernel that will always exist even when the module is not loaded.
> 
> Having said that, I am still going to make sure that it's not an
> issue with a previous patch for our driver. Although the shift to
> using udp_offload_get_port with this patch series will help alleviate
> that issue.
> 
> Anjali
> 
> > -----Original Message-----
> > From: Keller, Jacob E
> > Sent: Tuesday, November 10, 2015 10:01 AM
> > To: intel-wired-lan at lists.osuosl.org; Singhai, Anjali
> > Subject: Re: [Intel-wired-lan] [PATCH v4 1/6] i40e: Remove
> > CONFIG_I40E_VXLAN
> > 
> > Hi Anjali,
> > 
> > On Tue, 2015-11-10 at 09:56 -0800, Anjali Singhai Jain wrote:
> > > If the kernel flag CONFIG_VXLAN is true or CONFIG_VXLAN_MODULE is
> > > true, enable VXLAN offload in the driver.
> > > 
> > > v2: Fix bisection error for this patch series.
> > > 
> > > Signed-off-by: Kiran Patil <kiran.patil at intel.com>
> > > Signed-off-by: Anjali Singhai Jain <anjali.singhai at intel.com>
> > > ---
> > >  drivers/net/ethernet/intel/i40e/i40e.h      |  2 --
> > >  drivers/net/ethernet/intel/i40e/i40e_main.c | 10 ++--------
> > >  2 files changed, 2 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/intel/i40e/i40e.h
> > > b/drivers/net/ethernet/intel/i40e/i40e.h
> > > index 8ed759e..6c4d154 100644
> > > --- a/drivers/net/ethernet/intel/i40e/i40e.h
> > > +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> > > @@ -282,11 +282,9 @@ struct i40e_pf {
> > >  	u32 fd_atr_cnt;
> > >  	u32 fd_tcp_rule;
> > > 
> > > -#if IS_ENABLED(CONFIG_VXLAN)
> > >  	__be16  vxlan_ports[I40E_MAX_PF_UDP_OFFLOAD_PORTS];
> > >  	u16 pending_vxlan_bitmap;
> > > 
> > > -#endif
> > >  	enum i40e_interrupt_policy int_policy;
> > >  	u16 rx_itr_default;
> > >  	u16 tx_itr_default;
> > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > > b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > > index b447af6..0235a8a 100644
> > > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > > @@ -6990,13 +6990,13 @@ static void i40e_handle_mdd_event(struct
> > > i40e_pf *pf)
> > >  	i40e_flush(hw);
> > >  }
> > > 
> > > -#if IS_ENABLED(CONFIG_VXLAN)
> > >  /**
> > >   * i40e_sync_vxlan_filters_subtask - Sync the VSI filter list
> > > with HW
> > >   * @pf: board private structure
> > >   **/
> > >  static void i40e_sync_vxlan_filters_subtask(struct i40e_pf *pf)
> > >  {
> > > +#if IS_ENABLED(CONFIG_VXLAN)
> > 
> > Direct use of CONFIG_VXLAN is not going to work. What happens in
> > the case
> > where CONFIG_VXLAN=m and CONFIG_I40E=y?
> > 
> > The kernel will load i40e as it loads itself at boot and vxlan
> > module will not get
> > included (because i40e isn't a module so it doesn't run depmod etc)
> > and then
> > it will run and try to call vxlan code and fail because of an
> > unknown symbol
> > since vxlan won't have been started yet.
> > 
> > This is the entire reason why CONFIG_XXX_VXLAN exists. The
> > alternative is
> > to use select VXLAN and force VXLAN on when we enable I40E.
> > 
> > That's the primary reason why we can't directly use CONFIG_VXLAN as
> > a
> > check in driver code.
> > 
> > Regards,
> > Jake
> > 
> > >  	struct i40e_hw *hw = &pf->hw;
> > >  	i40e_status ret;
> > >  	__be16 port;
> > > @@ -7030,9 +7030,9 @@ static void
> > > i40e_sync_vxlan_filters_subtask(struct i40e_pf *pf)
> > >  			}
> > >  		}
> > >  	}
> > > +#endif
> > >  }
> > > 
> > > -#endif
> > >  /**
> > >   * i40e_service_task - Run the driver's async subtasks
> > >   * @work: pointer to work_struct containing our data @@ -7057,9
> > > +7057,7 @@ static void i40e_service_task(struct work_struct
> > > *work)
> > >  	i40e_watchdog_subtask(pf);
> > >  	i40e_fdir_reinit_subtask(pf);
> > >  	i40e_sync_filters_subtask(pf);
> > > -#if IS_ENABLED(CONFIG_VXLAN)
> > >  	i40e_sync_vxlan_filters_subtask(pf);
> > > -#endif
> > >  	i40e_clean_adminq_subtask(pf);
> > > 
> > >  	i40e_service_event_complete(pf);
> > > @@ -8433,7 +8431,6 @@ static int i40e_set_features(struct
> > > net_device
> > > *netdev,
> > >  	return 0;
> > >  }
> > > 
> > > -#if IS_ENABLED(CONFIG_VXLAN)
> > >  /**
> > >   * i40e_get_vxlan_port_idx - Lookup a possibly offloaded for Rx
> > > UDP
> > > port
> > >   * @pf: board private structure
> > > @@ -8528,7 +8525,6 @@ static void i40e_del_vxlan_port(struct
> > > net_device *netdev,
> > >  	}
> > >  }
> > > 
> > > -#endif
> > >  static int i40e_get_phys_port_id(struct net_device *netdev,
> > >  				 struct netdev_phys_item_id
> > > *ppid)
> > >  {
> > > @@ -8753,10 +8749,8 @@ static const struct net_device_ops
> > > i40e_netdev_ops = {
> > >  	.ndo_get_vf_config	= i40e_ndo_get_vf_config,
> > >  	.ndo_set_vf_link_state	=
> > > i40e_ndo_set_vf_link_state,
> > >  	.ndo_set_vf_spoofchk	= i40e_ndo_set_vf_spoofchk,
> > > -#if IS_ENABLED(CONFIG_VXLAN)
> > >  	.ndo_add_vxlan_port	= i40e_add_vxlan_port,
> > >  	.ndo_del_vxlan_port	= i40e_del_vxlan_port,
> > > -#endif
> > >  	.ndo_get_phys_port_id	= i40e_get_phys_port_id,
> > >  	.ndo_fdb_add		= i40e_ndo_fdb_add,
> > >  	.ndo_features_check	= i40e_features_check,