Re: [dpdk-dev] [PATCH v6 5/5] net/enetfec: add features

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 10/21/2021 5:47 AM, Apeksha Gupta wrote:
> This patch adds checksum and VLAN offloads in enetfec network
> poll mode driver.
> 
> Signed-off-by: Sachin Saxena <sachin.saxena@nxp.com>
> Signed-off-by: Apeksha Gupta <apeksha.gupta@nxp.com>

<...>

> @@ -611,9 +615,20 @@ static int
>   enetfec_eth_init(struct rte_eth_dev *dev)
>   {
>   	struct enetfec_private *fep = dev->data->dev_private;
> +	struct rte_eth_conf *eth_conf = &fep->dev->data->dev_conf;
> +	uint64_t rx_offloads = eth_conf->rxmode.offloads;
>   
>   	fep->full_duplex = FULL_DUPLEX;
>   	dev->dev_ops = &enetfec_ops;
> +	if (fep->quirks & QUIRK_VLAN)
> +		/* enable hw VLAN support */
> +		rx_offloads |= DEV_RX_OFFLOAD_VLAN;
> +
> +	if (fep->quirks & QUIRK_CSUM) {
> +		/* enable hw accelerator */
> +		rx_offloads |= DEV_RX_OFFLOAD_CHECKSUM;
> +		fep->flag_csum |= RX_FLAG_CSUM_EN;
> +	}

Driver is force enabling these Rx offloads even user is not asking for them?
Is it because HW doesn't support disabling them?
If it is configurable it should honor user configuration,
if not configurable please document as limitation.

<...>

> +
> +		if (rxq->fep->bufdesc_ex &&
> +			(rxq->fep->flag_csum & RX_FLAG_CSUM_EN)) {
> +			if ((rte_read32(&ebdp->bd_esc) &
> +				rte_cpu_to_le_32(RX_FLAG_CSUM_ERR)) == 0) {
> +				/* don't check it */
> +				mbuf->ol_flags = PKT_RX_IP_CKSUM_BAD;

warning: "PKT_RX_IP_CKSUM_BAD" is deprecated

> +			} else {
> +				mbuf->ol_flags = PKT_RX_IP_CKSUM_GOOD;

warning: "PKT_RX_IP_CKSUM_GOOD" is deprecated

> +			}
> +		}
> +
> +		/* Handle received VLAN packets */
> +		if (vlan_packet_rcvd) {
> +			mbuf->vlan_tci = vlan_tag;
> +			mbuf->ol_flags |= PKT_RX_VLAN_STRIPPED | PKT_RX_VLAN;

warning: "PKT_RX_VLAN_STRIPPED" is deprecated
warning: "PKT_RX_VLAN" is deprecated

> +		}
> +
>   		rxq->rx_mbuf[index] = new_mbuf;
>   		rte_write32(rte_cpu_to_le_32(rte_pktmbuf_iova(new_mbuf)),
>   				&bdp->bd_bufaddr);
> @@ -411,6 +458,10 @@ enetfec_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
>   
>   		if (txq->fep->bufdesc_ex) {
>   			struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp;
> +
> +			if (mbuf->ol_flags == PKT_RX_IP_CKSUM_GOOD)

Why checking Rx flag on the transmit function? Is it typo?