On 5/1/2020 12:33 PM, Ferruh Yigit wrote:
> On 4/30/2020 7:59 AM, ssar...@amd.com wrote:
>> From: Sardar Shamsher Singh <shamshersingh.sar...@amd.com>
>>
>> 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
>>
>> Signed-off-by: Sardar Shamsher Singh <shamshersingh.sar...@amd.com>
>
> <...>
>
>> +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 & ETH_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 & ETH_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 & ETH_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);
>> + /* 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.
>
> <...>
>> @@ -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.
>> + 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;
> }
> }
>
> <...>
>
>> @@ -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;
>> @@ -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.
>
