Re: [dpdk-dev] [PATCH v2 1/3] fm10k: enable FTAG based forwarding

["Wang, Xiao W" <xiao.w.wang@intel.com>]

Hi Mark,

> -----Original Message-----
> From: Chen, Jing D
> Sent: Wednesday, February 3, 2016 2:53 PM
> To: Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: dev@dpdk.org; Qiu, Michael <michael.qiu@intel.com>; He, Shaopeng
> <shaopeng.he@intel.com>
> Subject: RE: [PATCH v2 1/3] fm10k: enable FTAG based forwarding
> 
> Hi,
> 
> Best Regards,
> Mark
> 
> 
> > -----Original Message-----
> > From: Wang, Xiao W
> > Sent: Tuesday, February 02, 2016 6:50 PM
> > To: Chen, Jing D
> > Cc: dev@dpdk.org; Qiu, Michael; He, Shaopeng; Wang, Xiao W
> > Subject: [PATCH v2 1/3] fm10k: enable FTAG based forwarding
> >
> > This patch enables reading sglort info into mbuf for RX and inserting
> > an FTAG at the beginning of the packet for TX. The vlan_tci_outer
> > field selected from rte_mbuf structure for sglort is not used in fm10k now.
> > In FTAG based forwarding mode, the switch will forward packets
> > according to glort info in FTAG rather than mac and vlan table.
> >
> > To activate this feature, user needs to turn
> > ``CONFIG_RTE_LIBRTE_FM10K_FTAG_FWD``
> > to y in common_linuxapp or common_bsdapp. Currently this feature is
> > supported only on PF, because FM10K_PFVTCTL register is read-only for
> > VF.
> >
> > Signed-off-by: Wang Xiao W <xiao.w.wang@intel.com>
> > ---
> >  config/common_bsdapp               |  1 +
> >  config/common_linuxapp             |  1 +
> >  drivers/net/fm10k/fm10k_ethdev.c   | 12 ++++++++++++
> >  drivers/net/fm10k/fm10k_rxtx.c     | 17 +++++++++++++++++
> >  drivers/net/fm10k/fm10k_rxtx_vec.c |  9 +++++++++
> >  5 files changed, 40 insertions(+)
> >
> > diff --git a/config/common_bsdapp b/config/common_bsdapp index
> > ed7c31c..451f81a 100644
> > --- a/config/common_bsdapp
> > +++ b/config/common_bsdapp
> > @@ -208,6 +208,7 @@ CONFIG_RTE_LIBRTE_FM10K_DEBUG_TX=n
> >  CONFIG_RTE_LIBRTE_FM10K_DEBUG_TX_FREE=n
> >  CONFIG_RTE_LIBRTE_FM10K_DEBUG_DRIVER=n
> >  CONFIG_RTE_LIBRTE_FM10K_RX_OLFLAGS_ENABLE=y
> > +CONFIG_RTE_LIBRTE_FM10K_FTAG_FWD=n
> >
> >  #
> >  # Compile burst-oriented Mellanox ConnectX-3 (MLX4) PMD diff --git
> > a/config/common_linuxapp b/config/common_linuxapp index
> > 74bc515..c928bce 100644
> > --- a/config/common_linuxapp
> > +++ b/config/common_linuxapp
> > @@ -207,6 +207,7 @@ CONFIG_RTE_LIBRTE_FM10K_DEBUG_TX_FREE=n
> >  CONFIG_RTE_LIBRTE_FM10K_DEBUG_DRIVER=n
> >  CONFIG_RTE_LIBRTE_FM10K_RX_OLFLAGS_ENABLE=y
> >  CONFIG_RTE_LIBRTE_FM10K_INC_VECTOR=y
> > +CONFIG_RTE_LIBRTE_FM10K_FTAG_FWD=n
> >
> >  #
> >  # Compile burst-oriented Mellanox ConnectX-3 (MLX4) PMD diff --git
> > a/drivers/net/fm10k/fm10k_ethdev.c
> > b/drivers/net/fm10k/fm10k_ethdev.c
> > index e4aed94..3a15c24 100644
> > --- a/drivers/net/fm10k/fm10k_ethdev.c
> > +++ b/drivers/net/fm10k/fm10k_ethdev.c
> > @@ -668,6 +668,18 @@ fm10k_dev_tx_init(struct rte_eth_dev *dev)
> >  			PMD_INIT_LOG(ERR, "failed to disable queue %d", i);
> >  			return -1;
> >  		}
> > +#ifdef RTE_LIBRTE_FM10K_FTAG_FWD
> > +		/* Enable use of FTAG bit in TX descriptor, PFVTCTL
> > +		 * register is read-only for VF.
> > +		 */
> > +		if (hw->mac.type == fm10k_mac_pf)
> > +			FM10K_WRITE_REG(hw, FM10K_PFVTCTL(i),
> > +
> > 	FM10K_PFVTCTL_FTAG_DESC_ENABLE);
> > +		else {
> > +			PMD_INIT_LOG(ERR, "FTAG is not supported in
> > VF.\n");
> 
> "\n" is not necessary.

Yes, PMD_INIT_LOG has already included a "\n". I will remove it.

> 
> > +			return -1;
> 
> Return "-ENOTSUP"?
> 

Yes, "-ENOTSUP" is meaningful, thanks for your comments, I'll fix it in v3.

Best Regards,
Wang Xiao

> > +		}
> > +#endif
> >
> >  		/* set location and size for descriptor ring */
> >  		FM10K_WRITE_REG(hw, FM10K_TDBAL(i),