net/axgbe: enabling VLAN support in axgbe
PATCH v3 1/1 net/axgbe: enabling VLAN support in axgbe
From: Sardar Shamsher Singh <[email protected]>
adding below APIs for axgbe
- axgbe_enable_rx_vlan_stripping: to enable vlan header stipping
- axgbe_disable_rx_vlan_stripping: to disable vlan header stipping
- axgbe_enable_rx_vlan_filtering: to enable vlan filter mode
- axgbe_disable_rx_vlan_filtering: to disable vlan filter mode
- axgbe_update_vlan_hash_table: crc calculation and hash table update based on vlan values post filter enable
- axgbe_vlan_filter_set: setting of active vlan out of max 4K values before doing hash update of same
- axgbe_vlan_tpid_set: setting of default tpid values
- axgbe_vlan_offload_set: a top layer function to call stip/filter etc based on mask values
--- doc/guides/nics/features/axgbe.ini | 1 + drivers/net/axgbe/axgbe_common.h | 29 +++++ drivers/net/axgbe/axgbe_dev.c | 171 ++++++++++++++++++++++++++-- drivers/net/axgbe/axgbe_ethdev.c | 173 ++++++++++++++++++++++++++++- drivers/net/axgbe/axgbe_ethdev.h | 16 +++ drivers/net/axgbe/axgbe_rxtx.c | 63 ++++++++++- 6 files changed, 437 insertions(+), 16 deletions(-)
doc/guides/nics/features/axgbe.ini +1 -0
@@ -11,6 +11,7 @@ Scattered Rx = Y Promiscuous mode = Y Allmulticast mode = Y RSS hash = Y +VLAN = Y CRC offload = Y L3 checksum offload = Y L4 checksum offload = Y
drivers/net/axgbe/axgbe_common.h +29 -0
@@ -257,6 +257,7 @@#define MAC_HWF0R 0x011c#define MAC_HWF1R 0x0120#define MAC_HWF2R 0x0124 +#define MAC_HWF3R 0x0128#define MAC_MDIOSCAR 0x0200#define MAC_MDIOSCCDR 0x0204#define MAC_MDIOISR 0x0214
@@ -282,6 +283,18 @@#define MAC_TXSNR 0x0d30#define MAC_TXSSR 0x0d34 +/*VLAN control bit mask*/ +#define AXGBE_VLNCTRL_MASK 0x0000FFFF +#define VLAN_PRIO_MASK 0xe000/* Priority Code Point */ +#define VLAN_PRIO_SHIFT 13 +#define VLAN_CFI_MASK 0x1000/* Canonical Format Indicator */ +#define VLAN_TAG_PRESENT VLAN_CFI_MASK +#define VLAN_VID_MASK 0x0fff/* VLAN Identifier */ +#define VLAN_N_VID 4096 +#define VLAN_TABLE_SIZE 64 +#define VLAN_TABLE_BIT (vlan_id) (1UL << ((vlan_id) & 0x3F)) +#define VLAN_TABLE_IDX (vlan_id) ((vlan_id) >> 6) +#define MAC_QTFCR_INC 4#define MAC_MACA_INC 4#define MAC_HTR_INC 4
@@ -359,6 +372,10 @@#define MAC_HWF2R_TXCHCNT_WIDTH 4#define MAC_HWF2R_TXQCNT_INDEX 6#define MAC_HWF2R_TXQCNT_WIDTH 4 +#define MAC_HWF3R_CBTISEL_INDEX 4 +#define MAC_HWF3R_CBTISEL_WIDTH 1 +#define MAC_HWF3R_NRVF_INDEX 0 +#define MAC_HWF3R_NRVF_WIDTH 3#define MAC_IER_TSIE_INDEX 12#define MAC_IER_TSIE_WIDTH 1#define MAC_ISR_MMCRXIS_INDEX 9
@@ -513,6 +530,8 @@#define MAC_VLANIR_VLTI_WIDTH 1#define MAC_VLANIR_CSVL_INDEX 19#define MAC_VLANIR_CSVL_WIDTH 1 +#define MAC_VLANIR_VLC_INDEX 16 +#define MAC_VLANIR_VLC_WIDTH 2#define MAC_VLANTR_DOVLTC_INDEX 20#define MAC_VLANTR_DOVLTC_WIDTH 1#define MAC_VLANTR_ERSVLM_INDEX 19
@@ -523,12 +542,18 @@#define MAC_VLANTR_ETV_WIDTH 1#define MAC_VLANTR_EVLS_INDEX 21#define MAC_VLANTR_EVLS_WIDTH 2 +#define MAC_VLANTR_EIVLS_INDEX 21 +#define MAC_VLANTR_EIVLS_WIDTH 2#define MAC_VLANTR_EVLRXS_INDEX 24#define MAC_VLANTR_EVLRXS_WIDTH 1 +#define MAC_VLANTR_EIVLRXS_INDEX 31 +#define MAC_VLANTR_EIVLRXS_WIDTH 1#define MAC_VLANTR_VL_INDEX 0#define MAC_VLANTR_VL_WIDTH 16#define MAC_VLANTR_VTHM_INDEX 25#define MAC_VLANTR_VTHM_WIDTH 1 +#define MAC_VLANTR_EDVLP_INDEX 26 +#define MAC_VLANTR_EDVLP_WIDTH 1#define MAC_VLANTR_VTIM_INDEX 17#define MAC_VLANTR_VTIM_WIDTH 1#define MAC_VR_DEVID_INDEX 8
@@ -537,6 +562,10 @@#define MAC_VR_SNPSVER_WIDTH 8#define MAC_VR_USERVER_INDEX 16#define MAC_VR_USERVER_WIDTH 8 +#define MAC_VLANIR_VLT_INDEX 0 +#define MAC_VLANIR_VLT_WIDTH 16 +#define MAC_VLANTR_ERIVLT_INDEX 27 +#define MAC_VLANTR_ERIVLT_WIDTH 1/* MMC register offsets */ #define MMC_CR 0x0800
drivers/net/axgbe/axgbe_dev.c +161 -10
193 lines not on this page · raw message
drivers/net/axgbe/axgbe_ethdev.c +172 -1
216 lines not on this page · raw message
drivers/net/axgbe/axgbe_ethdev.h +16 -0
@@ -291,6 +291,13 @@ struct axgbe_hw_if {int (*config_tx_flow_control)(struct axgbe_port *);int (*config_rx_flow_control)(struct axgbe_port *); +/* vlan */ +int (*enable_rx_vlan_stripping)(struct axgbe_port *); +int (*disable_rx_vlan_stripping)(struct axgbe_port *); +int (*enable_rx_vlan_filtering)(struct axgbe_port *); +int (*disable_rx_vlan_filtering)(struct axgbe_port *); +int (*update_vlan_hash_table)(struct axgbe_port *); +int (*exit)(struct axgbe_port *); };
@@ -425,6 +432,12 @@ struct axgbe_hw_features {unsigned int tx_ch_cnt;/* Number of DMA Transmit Channels */ unsigned int pps_out_num;/* Number of PPS outputs */ unsigned int aux_snap_num;/* Number of Aux snapshot inputs */ + +/* HW Feature Register3 */ +unsigned int tx_q_vlan_tag_ins;/* Queue/Channel based VLAN tag */ +/* insertion on Tx Enable */ +unsigned int no_of_vlan_extn;/* Number of Extended VLAN Tag */ +/* Filters Enabled */ };struct axgbe_version_data {
@@ -644,6 +657,9 @@ struct axgbe_port {unsigned int hash_table_count;unsigned int uc_hash_mac_addr;unsigned int uc_hash_table[AXGBE_MAC_HASH_TABLE_SIZE ]; + +/* Filtering support */ +unsigned long active_vlans[VLAN_TABLE_SIZE ]; };void axgbe_init_function_ptrs_dev (struct axgbe_hw_if *hw_if);
drivers/net/axgbe/axgbe_rxtx.c +58 -5
115 lines not on this page · raw message
Signature · 1 line
2.17.1
Quoted 16 lines · On 4/30/2020 7:59 AM, [email protected] wrote:
From: Sardar Shamsher Singh <[email protected]>
adding below APIs for axgbe
-axgbe_enable_rx_vlan_stripping: to enable vlan header stipping
-axgbe_disable_rx_vlan_stripping: to disable vlan header stipping
-axgbe_enable_rx_vlan_filtering: to enable vlan filter mode
-axgbe_disable_rx_vlan_filtering: to disable vlan filter mode
-axgbe_update_vlan_hash_table: crc calculation and hash table update based on vlan values post filter enable
-axgbe_vlan_filter_set: setting of active vlan out of max 4K values before doing hash update of same
-axgbe_vlan_tpid_set: setting of default tpid values
-axgbe_vlan_offload_set: a top layer function to call stip/filter etc based on mask valuesSigned-off-by Sardar Shamsher Singh <[email protected]>
<...>
Quoted 36 lines
drivers/net/axgbe/axgbe_ethdev.c +36
-0+static int +axgbe_vlan_offload_set (struct rte_eth_dev *dev,int mask) +{ +struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; +struct axgbe_port *pdata = dev->data->dev_private; + +/* Indicate that VLAN Tx CTAGs come from context descriptors */ +AXGMAC_IOWRITE_BITS (pdata,MAC_VLANIR ,CSVL ,0 ); +AXGMAC_IOWRITE_BITS (pdata,MAC_VLANIR ,VLTI ,1 ); + +if (mask Ð_VLAN_STRIP_MASK ) { +if (rxmode->offloads &DEV_RX_OFFLOAD_VLAN_STRIP ) { +PMD_DRV_LOG (DEBUG ,"Strip ON for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.enable_rx_vlan_stripping (pdata); + }else { +PMD_DRV_LOG (DEBUG ,"Strip OFF for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.disable_rx_vlan_stripping (pdata); + } + } +if (mask Ð_VLAN_FILTER_MASK ) { +if (rxmode->offloads &DEV_RX_OFFLOAD_VLAN_FILTER ) { +PMD_DRV_LOG (DEBUG ,"Filter ON for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.enable_rx_vlan_filtering (pdata); + }else { +PMD_DRV_LOG (DEBUG ,"Filter OFF for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.disable_rx_vlan_filtering (pdata); + } + } +if (mask Ð_VLAN_EXTEND_MASK ) { +if (rxmode->offloads &DEV_RX_OFFLOAD_VLAN_EXTEND ) { +PMD_DRV_LOG (DEBUG ,"enabling vlan extended mode\n" ); +axgbe_vlan_extend_enable (pdata);
drivers/net/axgbe/axgbe_ethdev.c axgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev) · line 1418
Quoted 10 lines
drivers/net/axgbe/axgbe_ethdev.c +10
-0+/* Set global registers with default ethertype*/ +axgbe_vlan_tpid_set (dev,ETH_VLAN_TYPE_OUTER , +RTE_ETHER_TYPE_VLAN ); +axgbe_vlan_tpid_set (dev,ETH_VLAN_TYPE_INNER , +RTE_ETHER_TYPE_VLAN ); + }else { +PMD_DRV_LOG (DEBUG ,"disabling vlan extended mode\n" ); +axgbe_vlan_extend_disable (pdata); + } + }
Is the intention here to enable disable QinQ stip, because enable/disable fucntions talks about qinq, if so 'ETH_QINQ_STRIP_MASK' should be used.
<...>
Quoted 5 lines
drivers/net/axgbe/axgbe_rxtx.c +1
-0@@ -275,6 +275,23 @@ axgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,/* Get the RSS hash */ if (AXGMAC_GET_BITS_LE (desc->write.desc3,RX_NORMAL_DESC3 ,RSV )) mbuf->hash.rss =rte_le_to_cpu_32 (desc->write.desc1); + etlt = AXGMAC_GET_BITS_LE(desc->write.desc3,
drivers/net/axgbe/axgbe_rxtx.c axgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, · line 288
Quoted 10 lines
drivers/net/axgbe/axgbe_rxtx.c +10
-0+ RX_NORMAL_DESC3, ETLT); +if (!err || !etlt) { +if (etlt ==0x09 && + (rxq->pdata->eth_dev->data->dev_conf.rxmode.offloads & +DEV_RX_OFFLOAD_VLAN_STRIP )) { + mbuf->ol_flags |= +PKT_RX_VLAN |PKT_RX_VLAN_STRIPPED ; + mbuf->vlan_tci = +AXGMAC_GET_BITS_LE (desc->write.desc0, +RX_NORMAL_DESC0 ,OVT );
I don't know HW capabilities, but if 'etlt == 0x09' means packet has VLAN tag, you can use it independent from strip, like below, up to you.
if (vlan) {
mbuf->ol_flags |= PKT_RX_VLAN;
mbuf->vlan_tci = xxx
if (vlan_stripped) {
mbuf->ol_flags |= PKT_RX_VLAN_STRIPPED;
}
}<...>
Quoted 9 lines
drivers/net/axgbe/axgbe_rxtx.c +1
-0@@ -487,6 +520,7 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,struct axgbe_tx_queue *txq;unsigned int tsize;const struct rte_memzone *tz; +uint64_t offloads; tx_desc = nb_desc; pdata = dev->data->dev_private;
drivers/net/axgbe/axgbe_rxtx.c int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, · line 544
Quoted 6 lines
drivers/net/axgbe/axgbe_rxtx.c +2
-1@@ -506,7 +540,8 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,if (!txq)return -ENOMEM ; txq->pdata = pdata; - + offloads = tx_conf->offloads | + txq->pdata->eth_dev->data->dev_conf.txmode.offloads;
As far as I can see PMD doesn't support queue specific offloads, so 'tx_conf->offloads' should be always 0.
And since there is no queue specific offload and this 'offloads' information is only used to set burst function, which is again only port bases (this will keep overwrite same info per each queue), why not do this check in the 'axgbe_dev_configure()' instead of per queue?
And I can see you may hit the problem becuase of VLAN offload but the issue seems generic, not directly related to VLAN support, and this can be separate fix I think.
Quoted 72 lines · Ferruh Yigit
Quoted 16 lines · On 4/30/2020 7:59 AM, [email protected] wrote:
From: Sardar Shamsher Singh <[email protected]>
adding below APIs for axgbe
-axgbe_enable_rx_vlan_stripping: to enable vlan header stipping
-axgbe_disable_rx_vlan_stripping: to disable vlan header stipping
-axgbe_enable_rx_vlan_filtering: to enable vlan filter mode
-axgbe_disable_rx_vlan_filtering: to disable vlan filter mode
-axgbe_update_vlan_hash_table: crc calculation and hash table update based on vlan values post filter enable
-axgbe_vlan_filter_set: setting of active vlan out of max 4K values before doing hash update of same
-axgbe_vlan_tpid_set: setting of default tpid values
-axgbe_vlan_offload_set: a top layer function to call stip/filter etc based on mask valuesSigned-off-by Sardar Shamsher Singh <[email protected]><...>
Quoted 36 lines
drivers/net/axgbe/axgbe_ethdev.c +36
-0+static int +axgbe_vlan_offload_set (struct rte_eth_dev *dev,int mask) +{ +struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; +struct axgbe_port *pdata = dev->data->dev_private; + +/* Indicate that VLAN Tx CTAGs come from context descriptors */ +AXGMAC_IOWRITE_BITS (pdata,MAC_VLANIR ,CSVL ,0 ); +AXGMAC_IOWRITE_BITS (pdata,MAC_VLANIR ,VLTI ,1 ); + +if (mask Ð_VLAN_STRIP_MASK ) { +if (rxmode->offloads &DEV_RX_OFFLOAD_VLAN_STRIP ) { +PMD_DRV_LOG (DEBUG ,"Strip ON for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.enable_rx_vlan_stripping (pdata); + }else { +PMD_DRV_LOG (DEBUG ,"Strip OFF for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.disable_rx_vlan_stripping (pdata); + } + } +if (mask Ð_VLAN_FILTER_MASK ) { +if (rxmode->offloads &DEV_RX_OFFLOAD_VLAN_FILTER ) { +PMD_DRV_LOG (DEBUG ,"Filter ON for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.enable_rx_vlan_filtering (pdata); + }else { +PMD_DRV_LOG (DEBUG ,"Filter OFF for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.disable_rx_vlan_filtering (pdata); + } + } +if (mask Ð_VLAN_EXTEND_MASK ) { +if (rxmode->offloads &DEV_RX_OFFLOAD_VLAN_EXTEND ) { +PMD_DRV_LOG (DEBUG ,"enabling vlan extended mode\n" ); +axgbe_vlan_extend_enable (pdata);drivers/net/axgbe/axgbe_ethdev.c axgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev) · line 1418
Quoted 10 lines
drivers/net/axgbe/axgbe_ethdev.c +10
-0+/* Set global registers with default ethertype*/ +axgbe_vlan_tpid_set (dev,ETH_VLAN_TYPE_OUTER , +RTE_ETHER_TYPE_VLAN ); +axgbe_vlan_tpid_set (dev,ETH_VLAN_TYPE_INNER , +RTE_ETHER_TYPE_VLAN ); + }else { +PMD_DRV_LOG (DEBUG ,"disabling vlan extended mode\n" ); +axgbe_vlan_extend_disable (pdata); + } + }Is the intention here to enable disable QinQ stip, because enable/disable fucntions talks about qinq, if so '
ETH_QINQ_STRIP_MASK' should be used.<...>
Quoted 10 lines
drivers/net/axgbe/axgbe_rxtx.c axgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, · line 283
Quoted 10 lines
drivers/net/axgbe/axgbe_rxtx.c +6
-0@@ -275,6 +275,23 @@ axgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,/* Get the RSS hash */ if (AXGMAC_GET_BITS_LE (desc->write.desc3,RX_NORMAL_DESC3 ,RSV )) mbuf->hash.rss =rte_le_to_cpu_32 (desc->write.desc1); + etlt =AXGMAC_GET_BITS_LE (desc->write.desc3, +RX_NORMAL_DESC3 ,ETLT ); +if (!err || !etlt) { +if (etlt ==0x09 && + (rxq->pdata->eth_dev->data->dev_conf.rxmode.offloads & +DEV_RX_OFFLOAD_VLAN_STRIP )) {
Also checking these offload flags only in the '.vlan_offload_set' dev_ops is not enough, user may be set these flags but not called the '.vlan_offload_set' at all, that is why configure() fucntion should do similar checks and configuration.
Quoted 47 lines
drivers/net/axgbe/axgbe_rxtx.c axgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, · line 288
Quoted 5 lines
drivers/net/axgbe/axgbe_rxtx.c +5
-0+ mbuf->ol_flags |= +PKT_RX_VLAN |PKT_RX_VLAN_STRIPPED ; + mbuf->vlan_tci = +AXGMAC_GET_BITS_LE (desc->write.desc0, +RX_NORMAL_DESC0 ,OVT );I don't know HW capabilities, but if 'etlt ==
0x09' means packet has VLAN tag, you can use it independent from strip, like below, up to you.if (vlan) { mbuf->ol_flags |= PKT_RX_VLAN; mbuf->vlan_tci = xxx if (vlan_stripped) { mbuf->ol_flags |= PKT_RX_VLAN_STRIPPED; } }<...>
Quoted 9 lines
drivers/net/axgbe/axgbe_rxtx.c +1
-0@@ -487,6 +520,7 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,struct axgbe_tx_queue *txq;unsigned int tsize;const struct rte_memzone *tz; +uint64_t offloads; tx_desc = nb_desc; pdata = dev->data->dev_private;drivers/net/axgbe/axgbe_rxtx.c int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, · line 544
Quoted 6 lines
drivers/net/axgbe/axgbe_rxtx.c +2
-1@@ -506,7 +540,8 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,if (!txq)return -ENOMEM ; txq->pdata = pdata; - + offloads = tx_conf->offloads | + txq->pdata->eth_dev->data->dev_conf.txmode.offloads;As far as I can see PMD doesn't support queue specific offloads, so '
tx_conf->offloads' should be always 0.And since there is no queue specific offload and this 'offloads' information is only used to set burst function, which is again only port bases (this will keep overwrite same info per each queue), why not do this check in the '
axgbe_dev_configure()' instead of per queue?And I can see you may hit the problem becuase of VLAN offload but the issue seems generic, not directly related to VLAN support, and this can be separate fix I think.
[AMD Official Use Only - Internal Distribution Only]
Hi Ferruh,
Thanks for knowledge sharing.
Yes etlt - 0x09 is nothing but indicate " ■ 4’b1001: The packet is type packet with Single CVLAN tag." And you are right it should be as below and will do changes on same:
if (vlan) {
mbuf->ol_flags |= PKT_RX_VLAN;
mbuf->vlan_tci = xxx
if (vlan_stripped) {
mbuf->ol_flags |= PKT_RX_VLAN_STRIPPED;
}
}But rest of the items will do as incremental development.
1. Like in axgbe_dev_configure() need to check for default setting when system comes up.
2. There are lot of changes need to be add for QinQ support.
Will add all as incremental development work.
Currently working to make other vendor's SFP to work and in parallel above changes will add for upstream. Hope this should be fine.
Can you please put some more light on below, what type issues may occur?
" And I can see you may hit the problem becuase of VLAN offload but the issue seems generic, not directly related to VLAN support, and this can be separate fix I think."
Thanks & regards
S Shamsher Singh
(+91)7259016141
-----Original Message-----
From: Ferruh Yigit <[email protected]>
Sent: Friday, May 1, 2020 5:04 PM
To: Sardar, Shamsher singh <[email protected]>; [email protected]
Subject: Re: [dpdk-dev] [PATCH v3] net/axgbe: enabling VLAN support in axgbe
[CAUTION: External Email]
Quoted 16 lines · On 4/30/2020 7:59 AM, [email protected] wrote:
From: Sardar Shamsher Singh <[email protected]>
adding below APIs for axgbe
-axgbe_enable_rx_vlan_stripping: to enable vlan header stipping
-axgbe_disable_rx_vlan_stripping: to disable vlan header stipping
-axgbe_enable_rx_vlan_filtering: to enable vlan filter mode
-axgbe_disable_rx_vlan_filtering: to disable vlan filter mode
-axgbe_update_vlan_hash_table: crc calculation and hash table update based on vlan values post filter enable
-axgbe_vlan_filter_set: setting of active vlan out of max 4K values before doing hash update of same
-axgbe_vlan_tpid_set: setting of default tpid values
-axgbe_vlan_offload_set: a top layer function to call stip/filter etc based on mask valuesSigned-off-by Sardar Shamsher Singh <[email protected]>
<...>
Quoted 35 lines
drivers/net/axgbe/axgbe_ethdev.c +35
-0+static int +axgbe_vlan_offload_set (struct rte_eth_dev *dev,int mask) { +struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; +struct axgbe_port *pdata = dev->data->dev_private; + +/* Indicate that VLAN Tx CTAGs come from context descriptors */ +AXGMAC_IOWRITE_BITS (pdata,MAC_VLANIR ,CSVL ,0 ); +AXGMAC_IOWRITE_BITS (pdata,MAC_VLANIR ,VLTI ,1 ); + +if (mask Ð_VLAN_STRIP_MASK ) { +if (rxmode->offloads &DEV_RX_OFFLOAD_VLAN_STRIP ) { +PMD_DRV_LOG (DEBUG ,"Strip ON for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.enable_rx_vlan_stripping (pdata); + }else { +PMD_DRV_LOG (DEBUG ,"Strip OFF for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.disable_rx_vlan_stripping (pdata); + } + } +if (mask Ð_VLAN_FILTER_MASK ) { +if (rxmode->offloads &DEV_RX_OFFLOAD_VLAN_FILTER ) { +PMD_DRV_LOG (DEBUG ,"Filter ON for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.enable_rx_vlan_filtering (pdata); + }else { +PMD_DRV_LOG (DEBUG ,"Filter OFF for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.disable_rx_vlan_filtering (pdata); + } + } +if (mask Ð_VLAN_EXTEND_MASK ) { +if (rxmode->offloads &DEV_RX_OFFLOAD_VLAN_EXTEND ) { +PMD_DRV_LOG (DEBUG ,"enabling vlan extended mode\n" ); +axgbe_vlan_extend_enable (pdata);
drivers/net/axgbe/axgbe_ethdev.c axgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev) · line 1418
Quoted 10 lines
drivers/net/axgbe/axgbe_ethdev.c +10
-0+/* Set global registers with default ethertype*/ +axgbe_vlan_tpid_set (dev,ETH_VLAN_TYPE_OUTER , +RTE_ETHER_TYPE_VLAN ); +axgbe_vlan_tpid_set (dev,ETH_VLAN_TYPE_INNER , +RTE_ETHER_TYPE_VLAN ); + }else { +PMD_DRV_LOG (DEBUG ,"disabling vlan extended mode\n" ); +axgbe_vlan_extend_disable (pdata); + } + }
Is the intention here to enable disable QinQ stip, because enable/disable fucntions talks about qinq, if so 'ETH_QINQ_STRIP_MASK' should be used.
<...>
Quoted 7 lines
drivers/net/axgbe/axgbe_rxtx.c +2
-0@@ -275,6 +275,23 @@ axgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,/* Get the RSS hash */ if (AXGMAC_GET_BITS_LE (desc->write.desc3,RX_NORMAL_DESC3 ,RSV )) mbuf->hash.rss =rte_le_to_cpu_32 (desc->write.desc1); + etlt =AXGMAC_GET_BITS_LE (desc->write.desc3, +RX_NORMAL_DESC3 ,ETLT );
drivers/net/axgbe/axgbe_rxtx.c axgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, · line 290
Quoted 10 lines
drivers/net/axgbe/axgbe_rxtx.c +10
-0+if (!err || !etlt) { +if (etlt ==0x09 && + (rxq->pdata->eth_dev->data->dev_conf.rxmode.offloads & +DEV_RX_OFFLOAD_VLAN_STRIP )) { + mbuf->ol_flags |= +PKT_RX_VLAN |PKT_RX_VLAN_STRIPPED ; + mbuf->vlan_tci = +AXGMAC_GET_BITS_LE (desc->write.desc0, +RX_NORMAL_DESC0 , +OVT );
I don't know HW capabilities, but if 'etlt == 0x09' means packet has VLAN tag, you can use it independent from strip, like below, up to you.
if (vlan) {
mbuf->ol_flags |= PKT_RX_VLAN;
mbuf->vlan_tci = xxx
if (vlan_stripped) {
mbuf->ol_flags |= PKT_RX_VLAN_STRIPPED;
}
}<...>
Quoted 9 lines
drivers/net/axgbe/axgbe_rxtx.c +1
-0@@ -487,6 +520,7 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,struct axgbe_tx_queue *txq;unsigned int tsize;const struct rte_memzone *tz; +uint64_t offloads; tx_desc = nb_desc; pdata = dev->data->dev_private;
drivers/net/axgbe/axgbe_rxtx.c int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, · line 544
Quoted 6 lines
drivers/net/axgbe/axgbe_rxtx.c +2
-1@@ -506,7 +540,8 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,if (!txq)return -ENOMEM ; txq->pdata = pdata; - + offloads = tx_conf->offloads | + txq->pdata->eth_dev->data->dev_conf.txmode.offloads;
As far as I can see PMD doesn't support queue specific offloads, so 'tx_conf->offloads' should be always 0.
And since there is no queue specific offload and this 'offloads' information is only used to set burst function, which is again only port bases (this will keep overwrite same info per each queue), why not do this check in the 'axgbe_dev_configure()' instead of per queue?
And I can see you may hit the problem becuase of VLAN offload but the issue seems generic, not directly related to VLAN support, and this can be separate fix I think.
Quoted 15 lines · Shamsher singh
[AMD Official Use Only - Internal Distribution Only]
Hi Ferruh,
Thanks for knowledge sharing.
Yes etlt -0x09is nothing but indicate " ■ 4’b1001: The packet is type packet with Single CVLAN tag." And you are right it should be as below and will do changes on same:if (vlan) { mbuf->ol_flags |= PKT_RX_VLAN; mbuf->vlan_tci = xxx if (vlan_stripped) { mbuf->ol_flags |= PKT_RX_VLAN_STRIPPED; } }
Quoted 2 lines
But rest of the items will do as incremental development.
1. Like inaxgbe_dev_configure()need to check for default setting when system comes up.
OK
Quoted 1 line
2. There are lot of changes need to be add for QinQ support.
I just would like to be sure you are not confusing QinQ support with 'DEV_RX_OFFLOAD_VLAN_EXTEND', because 'DEV_RX_OFFLOAD_VLAN_EXTEND' is not very clearly defined.
In the 'axgbe_vlan_extend_enable()', it mentions from 'qinq', so it is confusing. If you are clear on what 'DEV_RX_OFFLOAD_VLAN_EXTEND' is, that is OK. If not I sugggest dropping the 'ETH_VLAN_EXTEND_MASK' part.
Quoted 3 lines
Will add all as incremental development work.
Currently working to make other vendor's SFP to work and in parallel above changes will add for upstream. Hope this should be fine.
OK
Quoted 3 lines
Can you please put some more light on below, what type issues may occur?
" And I can see you may hit the problem becuase of VLAN offload but the issue seems generic, not directly related to VLAN support, and this can be separate fix I think."
I was refering to the changes in 'axgbe_dev_tx_queue_setup()',
a) Why those changes are done at all?
b) Why they are done in this patch? Should it be a seperate fix?
Quoted 135 lines
Thanks & regards
S Shamsher Singh
(+91)7259016141-----Original Message-----
From: Ferruh Yigit <[email protected]>
Sent: Friday, May 1, 2020 5:04 PM
To: Sardar, Shamsher singh <[email protected]>; [email protected]
Subject: Re: [dpdk-dev] [PATCH v3] net/axgbe: enabling VLAN support in axgbe[CAUTION: External Email]
Quoted 16 lines · On 4/30/2020 7:59 AM, [email protected] wrote:
From: Sardar Shamsher Singh <[email protected]>
adding below APIs for axgbe
-axgbe_enable_rx_vlan_stripping: to enable vlan header stipping
-axgbe_disable_rx_vlan_stripping: to disable vlan header stipping
-axgbe_enable_rx_vlan_filtering: to enable vlan filter mode
-axgbe_disable_rx_vlan_filtering: to disable vlan filter mode
-axgbe_update_vlan_hash_table: crc calculation and hash table update based on vlan values post filter enable
-axgbe_vlan_filter_set: setting of active vlan out of max 4K values before doing hash update of same
-axgbe_vlan_tpid_set: setting of default tpid values
-axgbe_vlan_offload_set: a top layer function to call stip/filter etc based on mask valuesSigned-off-by Sardar Shamsher Singh <[email protected]><...>
Quoted 35 lines
drivers/net/axgbe/axgbe_ethdev.c +35
-0+static int +axgbe_vlan_offload_set (struct rte_eth_dev *dev,int mask) { +struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; +struct axgbe_port *pdata = dev->data->dev_private; + +/* Indicate that VLAN Tx CTAGs come from context descriptors */ +AXGMAC_IOWRITE_BITS (pdata,MAC_VLANIR ,CSVL ,0 ); +AXGMAC_IOWRITE_BITS (pdata,MAC_VLANIR ,VLTI ,1 ); + +if (mask Ð_VLAN_STRIP_MASK ) { +if (rxmode->offloads &DEV_RX_OFFLOAD_VLAN_STRIP ) { +PMD_DRV_LOG (DEBUG ,"Strip ON for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.enable_rx_vlan_stripping (pdata); + }else { +PMD_DRV_LOG (DEBUG ,"Strip OFF for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.disable_rx_vlan_stripping (pdata); + } + } +if (mask Ð_VLAN_FILTER_MASK ) { +if (rxmode->offloads &DEV_RX_OFFLOAD_VLAN_FILTER ) { +PMD_DRV_LOG (DEBUG ,"Filter ON for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.enable_rx_vlan_filtering (pdata); + }else { +PMD_DRV_LOG (DEBUG ,"Filter OFF for device = %s\n" , + pdata->eth_dev->device->name); + pdata->hw_if.disable_rx_vlan_filtering (pdata); + } + } +if (mask Ð_VLAN_EXTEND_MASK ) { +if (rxmode->offloads &DEV_RX_OFFLOAD_VLAN_EXTEND ) { +PMD_DRV_LOG (DEBUG ,"enabling vlan extended mode\n" ); +axgbe_vlan_extend_enable (pdata);drivers/net/axgbe/axgbe_ethdev.c axgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev) · line 1418
Quoted 10 lines
drivers/net/axgbe/axgbe_ethdev.c +10
-0+/* Set global registers with default ethertype*/ +axgbe_vlan_tpid_set (dev,ETH_VLAN_TYPE_OUTER , +RTE_ETHER_TYPE_VLAN ); +axgbe_vlan_tpid_set (dev,ETH_VLAN_TYPE_INNER , +RTE_ETHER_TYPE_VLAN ); + }else { +PMD_DRV_LOG (DEBUG ,"disabling vlan extended mode\n" ); +axgbe_vlan_extend_disable (pdata); + } + }Is the intention here to enable disable QinQ stip, because enable/disable fucntions talks about qinq, if so '
ETH_QINQ_STRIP_MASK' should be used.<...>
Quoted 7 lines
drivers/net/axgbe/axgbe_rxtx.c +2
-0@@ -275,6 +275,23 @@ axgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,/* Get the RSS hash */ if (AXGMAC_GET_BITS_LE (desc->write.desc3,RX_NORMAL_DESC3 ,RSV )) mbuf->hash.rss =rte_le_to_cpu_32 (desc->write.desc1); + etlt =AXGMAC_GET_BITS_LE (desc->write.desc3, +RX_NORMAL_DESC3 ,ETLT );drivers/net/axgbe/axgbe_rxtx.c axgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, · line 290
Quoted 10 lines
drivers/net/axgbe/axgbe_rxtx.c +10
-0+if (!err || !etlt) { +if (etlt ==0x09 && + (rxq->pdata->eth_dev->data->dev_conf.rxmode.offloads & +DEV_RX_OFFLOAD_VLAN_STRIP )) { + mbuf->ol_flags |= +PKT_RX_VLAN |PKT_RX_VLAN_STRIPPED ; + mbuf->vlan_tci = +AXGMAC_GET_BITS_LE (desc->write.desc0, +RX_NORMAL_DESC0 , +OVT );I don't know HW capabilities, but if 'etlt ==
0x09' means packet has VLAN tag, you can use it independent from strip, like below, up to you.if (vlan) { mbuf->ol_flags |= PKT_RX_VLAN; mbuf->vlan_tci = xxx if (vlan_stripped) { mbuf->ol_flags |= PKT_RX_VLAN_STRIPPED; } }<...>
Quoted 9 lines
drivers/net/axgbe/axgbe_rxtx.c +1
-0@@ -487,6 +520,7 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,struct axgbe_tx_queue *txq;unsigned int tsize;const struct rte_memzone *tz; +uint64_t offloads; tx_desc = nb_desc; pdata = dev->data->dev_private;drivers/net/axgbe/axgbe_rxtx.c int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, · line 544
Quoted 6 lines
drivers/net/axgbe/axgbe_rxtx.c +2
-1@@ -506,7 +540,8 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,if (!txq)return -ENOMEM ; txq->pdata = pdata; - + offloads = tx_conf->offloads | + txq->pdata->eth_dev->data->dev_conf.txmode.offloads;As far as I can see PMD doesn't support queue specific offloads, so '
tx_conf->offloads' should be always 0.And since there is no queue specific offload and this 'offloads' information is only used to set burst function, which is again only port bases (this will keep overwrite same info per each queue), why not do this check in the '
axgbe_dev_configure()' instead of per queue?And I can see you may hit the problem becuase of VLAN offload but the issue seems generic, not directly related to VLAN support, and this can be separate fix I think.