netdev - Re: [PATCH] net: cxgb4{,vf}: convert to hw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4DA895DF.1090809@chelsio.com>
Date:	Fri, 15 Apr 2011 12:00:47 -0700
From:	Dimitris Michailidis <dm@...lsio.com>
To:	Michał Mirosław <mirq-linux@...e.qmqm.pl>
CC:	netdev@...r.kernel.org, Casey Leedom <leedom@...lsio.com>
Subject: Re: [PATCH] net: cxgb4{,vf}: convert to hw_features

Michał Mirosław wrote:

> +#define TSO_FLAGS (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
>  #define VLAN_FEAT (NETIF_F_SG | NETIF_F_IP_CSUM | TSO_FLAGS | \
>  		   NETIF_F_IPV6_CSUM | NETIF_F_HIGHDMA)
>  
> @@ -3665,14 +3627,14 @@ static int __devinit init_one(struct pci_dev *pdev,
>  		pi = netdev_priv(netdev);
>  		pi->adapter = adapter;
>  		pi->xact_addr_filt = -1;
> -		pi->rx_offload = RX_CSO;
>  		pi->port_id = i;
>  		netdev->irq = pdev->irq;
>  
> -		netdev->features |= NETIF_F_SG | TSO_FLAGS;
> -		netdev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> -		netdev->features |= NETIF_F_GRO | NETIF_F_RXHASH | highdma;
> -		netdev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
> +		netdev->hw_features = NETIF_F_SG | TSO_FLAGS |
> +			NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> +			NETIF_F_RXCSUM | NETIF_F_RXHASH |
> +			NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
> +		netdev->features |= netdev->hw_features | highdma;
>  		netdev->vlan_features = netdev->features & VLAN_FEAT;

Here vlan_features does not include NETIF_F_RXCSUM but the cxgb4vf bits 
below do include it.  I looked at some other drivers and saw again some 
include it and some don't.  The core VLAN code handles NETIF_F_RXCSUM on its 
own.  Is there some rule for whether drivers should set it in their 
vlan_features or not?

> diff --git a/drivers/net/cxgb4/sge.c b/drivers/net/cxgb4/sge.c
> index 311471b..e8f6f8e 100644
> --- a/drivers/net/cxgb4/sge.c
> +++ b/drivers/net/cxgb4/sge.c
> @@ -1587,7 +1587,7 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp,
>  	pi = netdev_priv(skb->dev);
>  	rxq->stats.pkts++;
>  
> -	if (csum_ok && (pi->rx_offload & RX_CSO) &&
> +	if (csum_ok && (q->netdev->features & NETIF_F_RXCSUM) &&
>  	    (pkt->l2info & htonl(RXF_UDP | RXF_TCP))) {
>  		if (!pkt->ip_frag) {
>  			skb->ip_summed = CHECKSUM_UNNECESSARY;

With this change variable 'pi' can be removed but I can do this cleanup 
after this patch goes in.

> diff --git a/drivers/net/cxgb4vf/cxgb4vf_main.c b/drivers/net/cxgb4vf/cxgb4vf_main.c
> index c662679..04a5c2d 100644
> --- a/drivers/net/cxgb4vf/cxgb4vf_main.c
> +++ b/drivers/net/cxgb4vf/cxgb4vf_main.c
> @@ -2638,14 +2597,13 @@ static int __devinit cxgb4vf_pci_probe(struct pci_dev *pdev,
>  		 * it.
>  		 */
>  		pi->xact_addr_filt = -1;
> -		pi->rx_offload = RX_CSO;
>  		netif_carrier_off(netdev);
>  		netdev->irq = pdev->irq;
>  
> -		netdev->features = (NETIF_F_SG | TSO_FLAGS |
> -				    NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> -				    NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX |
> -				    NETIF_F_GRO);
> +		netdev->hw_features = NETIF_F_SG | TSO_FLAGS | NETIF_F_RXCSUM |
> +			NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> +			NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
> +		netdev->features = netdev->hw_features;
>  		if (pci_using_dac)
>  			netdev->features |= NETIF_F_HIGHDMA;
>  		netdev->vlan_features =

cxgb4vf does not implement toggling of NETIF_F_HW_VLAN_RX so I think the 
flag should be set in features but not hw_features or maybe the driver needs 
to handle it in fix_features?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html