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

Singhai, Anjali anjali.singhai at intel.com
Tue Nov 10 19:08:57 UTC 2015


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,


More information about the Intel-wired-lan mailing list