Message-ID: <c7ec9d3d-c8da-0b9a-2420-fa9031074613@huawei.com>
Date: Thu, 11 May 2023 20:21:53 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Mengyuan Lou <mengyuanlou@...-swift.com>, <netdev@...r.kernel.org>
CC: <jiawenwu@...stnetic.com>
Subject: Re: [PATCH net-next v4 3/7] net: wangxun: Implement vlan add and kill
 functions

On 2023/5/10 17:38, Mengyuan Lou wrote:
> Implement vlan add/kill functions which add and remove
> vlan id in hardware.
> 
> Signed-off-by: Mengyuan Lou <mengyuanlou@...-swift.com>
> ---
>  drivers/net/ethernet/wangxun/libwx/wx_hw.c   | 275 ++++++++++++++++++-
>  drivers/net/ethernet/wangxun/libwx/wx_hw.h   |   3 +
>  drivers/net/ethernet/wangxun/libwx/wx_lib.c  |  18 ++
>  drivers/net/ethernet/wangxun/libwx/wx_type.h |  31 +++
>  4 files changed, 326 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.c b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> index ca409b4054d0..4b7baeb6c568 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> @@ -1182,12 +1182,30 @@ static void wx_enable_sec_rx_path(struct wx *wx)
>  	WX_WRITE_FLUSH(wx);
>  }
>  
> +static void wx_vlan_strip_control(struct wx *wx, bool enable)
> +{
> +	int i, j;
> +
> +	for (i = 0; i < wx->num_rx_queues; i++) {
> +		struct wx_ring *ring = wx->rx_ring[i];
> +
> +		if (ring->accel)
> +			continue;

Nit: add a blane line here?

> +		j = ring->reg_idx;
> +		wr32m(wx, WX_PX_RR_CFG(j), WX_PX_RR_CFG_VLAN,
> +		      enable ? WX_PX_RR_CFG_VLAN : 0);
> +	}
> +}
> +
>  void wx_set_rx_mode(struct net_device *netdev)
>  {
>  	struct wx *wx = netdev_priv(netdev);
> +	netdev_features_t features;
>  	u32 fctrl, vmolr, vlnctrl;
>  	int count;
>  
> +	features = netdev->features;
> +
>  	/* Check for Promiscuous and All Multicast modes */
>  	fctrl = rd32(wx, WX_PSR_CTL);
>  	fctrl &= ~(WX_PSR_CTL_UPE | WX_PSR_CTL_MPE);
> @@ -1254,6 +1272,13 @@ void wx_set_rx_mode(struct net_device *netdev)
>  	wr32(wx, WX_PSR_VLAN_CTL, vlnctrl);
>  	wr32(wx, WX_PSR_CTL, fctrl);
>  	wr32(wx, WX_PSR_VM_L2CTL(0), vmolr);
> +
> +	if ((features & NETIF_F_HW_VLAN_CTAG_RX) &&
> +	    (features & NETIF_F_HW_VLAN_STAG_RX))
> +		wx_vlan_strip_control(wx, true);
> +	else
> +		wx_vlan_strip_control(wx, false);

Is there any reason not check features bits in the
ndev->ndo_set_features?

> +
>  }
>  EXPORT_SYMBOL(wx_set_rx_mode);
>  
> @@ -1462,6 +1487,16 @@ static void wx_configure_tx(struct wx *wx)
>  	      WX_MAC_TX_CFG_TE, WX_MAC_TX_CFG_TE);
>  }
>  
> +static void wx_restore_vlan(struct wx *wx)
> +{
> +	u16 vid = 1;
> +
> +	wx_vlan_rx_add_vid(wx->netdev, htons(ETH_P_8021Q), 0);
> +
> +	for_each_set_bit_from(vid, wx->active_vlans, VLAN_N_VID)
> +		wx_vlan_rx_add_vid(wx->netdev, htons(ETH_P_8021Q), vid);
> +}
> +
>  /**
>   * wx_configure_rx - Configure Receive Unit after Reset
>   * @wx: pointer to private structure
> @@ -1527,7 +1562,7 @@ void wx_configure(struct wx *wx)
>  	wx_configure_port(wx);
>  
>  	wx_set_rx_mode(wx->netdev);
> -
> +	wx_restore_vlan(wx);
>  	wx_enable_sec_rx_path(wx);
>  
>  	wx_configure_tx(wx);
> @@ -1727,4 +1762,242 @@ int wx_sw_init(struct wx *wx)
>  }
>  EXPORT_SYMBOL(wx_sw_init);
>  
> +/**
> + *  wx_find_vlvf_slot - find the vlanid or the first empty slot
> + *  @wx: pointer to hardware structure
> + *  @vlan: VLAN id to write to VLAN filter
> + *
> + *  return the VLVF index where this VLAN id should be placed
> + *
> + **/
> +static int wx_find_vlvf_slot(struct wx *wx, u32 vlan)
> +{
> +	u32 bits = 0, first_empty_slot = 0;
> +	int regindex;
> +
> +	/* short cut the special case */
> +	if (vlan == 0)
> +		return 0;
> +
> +	/* Search for the vlan id in the VLVF entries. Save off the first empty
> +	 * slot found along the way
> +	 */
> +	for (regindex = 1; regindex < WX_PSR_VLAN_SWC_ENTRIES; regindex++) {
> +		wr32(wx, WX_PSR_VLAN_SWC_IDX, regindex);
> +		bits = rd32(wx, WX_PSR_VLAN_SWC);
> +		if (!bits && !(first_empty_slot))
> +			first_empty_slot = regindex;
> +		else if ((bits & 0x0FFF) == vlan)
> +			break;
> +	}
> +
> +	if (regindex >= WX_PSR_VLAN_SWC_ENTRIES) {
> +		if (first_empty_slot)
> +			regindex = first_empty_slot;
> +		else
> +			regindex = -ENOMEM;
> +	}
> +
> +	return regindex;
> +}
> +
> +/**
> + *  wx_set_vlvf - Set VLAN Pool Filter
> + *  @wx: pointer to hardware structure
> + *  @vlan: VLAN id to write to VLAN filter
> + *  @vind: VMDq output index that maps queue to VLAN id in VFVFB
> + *  @vlan_on: boolean flag to turn on/off VLAN in VFVF
> + *  @vfta_changed: pointer to boolean flag which indicates whether VFTA
> + *                 should be changed
> + *
> + *  Turn on/off specified bit in VLVF table.
> + **/
> +static int wx_set_vlvf(struct wx *wx, u32 vlan, u32 vind, bool vlan_on,
> +		       bool *vfta_changed)
> +{
> +	u32 vt;
> +
> +	/* If VT Mode is set
> +	 *   Either vlan_on
> +	 *     make sure the vlan is in VLVF
> +	 *     set the vind bit in the matching VLVFB
> +	 *   Or !vlan_on
> +	 *     clear the pool bit and possibly the vind
> +	 */
> +	vt = rd32(wx, WX_CFG_PORT_CTL);
> +	if (vt & WX_CFG_PORT_CTL_NUM_VT_MASK) {

Maybe reduce one indentation by:

	if(!vt & WX_CFG_PORT_CTL_NUM_VT_MASK)
		return;

> +		s32 vlvf_index;
> +		u32 bits;
> +
> +		vlvf_index = wx_find_vlvf_slot(wx, vlan);
> +		if (vlvf_index < 0)
> +			return vlvf_index;
> +
> +		wr32(wx, WX_PSR_VLAN_SWC_IDX, vlvf_index);
> +		if (vlan_on) {
> +			/* set the pool bit */
> +			if (vind < 32) {
> +				bits = rd32(wx, WX_PSR_VLAN_SWC_VM_L);
> +				bits |= (1 << vind);
> +				wr32(wx, WX_PSR_VLAN_SWC_VM_L, bits);
> +			} else {
> +				bits = rd32(wx, WX_PSR_VLAN_SWC_VM_H);
> +				bits |= (1 << (vind - 32));
> +				wr32(wx, WX_PSR_VLAN_SWC_VM_H, bits);
> +			}
> +		} else {
> +			/* clear the pool bit */
> +			if (vind < 32) {
> +				bits = rd32(wx, WX_PSR_VLAN_SWC_VM_L);
> +				bits &= ~(1 << vind);
> +				wr32(wx, WX_PSR_VLAN_SWC_VM_L, bits);
> +				bits |= rd32(wx, WX_PSR_VLAN_SWC_VM_H);
> +			} else {
> +				bits = rd32(wx, WX_PSR_VLAN_SWC_VM_H);
> +				bits &= ~(1 << (vind - 32));
> +				wr32(wx, WX_PSR_VLAN_SWC_VM_H, bits);
> +				bits |= rd32(wx, WX_PSR_VLAN_SWC_VM_L);
> +			}
> +		}
> +
> +		if (bits) {
> +			wr32(wx, WX_PSR_VLAN_SWC, (WX_PSR_VLAN_SWC_VIEN | vlan));
> +			if (!vlan_on && vfta_changed)
> +				*vfta_changed = false;
> +		} else {
> +			wr32(wx, WX_PSR_VLAN_SWC, 0);
> +		}
> +	}
> +
> +	return 0;
> +}
> +

...

> +
>  struct wx_ring {
>  	struct wx_ring *next;           /* pointer to next ring in q_vector */
>  	struct wx_q_vector *q_vector;   /* backpointer to host q_vector */
>  	struct net_device *netdev;      /* netdev ring belongs to */
>  	struct device *dev;             /* device for DMA mapping */
> +	struct wx_fwd_adapter *accel;

It seems accel is not really necessary for this patch, as
it is only checked wx_vlan_strip_control().

Powered by blists - more mailing lists