| 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
| ||
|
Date: Thu, 07 Apr 2011 12:52:45 -0700
From: roprabhu <roprabhu@...co.com>
To: Michał Mirosław
<mirq-linux@...e.qmqm.pl>, <netdev@...r.kernel.org>
CC: Christian Benvenuti <benve@...co.com>,
Vasanthy Kolluri <vkolluri@...co.com>,
David Wang <dwang2@...co.com>
Subject: Re: [PATCH] net: enic: convert to hw_features
Thanks michal.
One small comment below,
On 4/7/11 5:43 AM, "Michał Mirosław" <mirq-linux@...e.qmqm.pl> wrote:
> As the driver uses GRO and not LRO, LRO settings are ignored anyway
> and are removed here to avoid confusion.
>
> Signed-off-by: Michał Mirosław <mirq-linux@...e.qmqm.pl>
> ---
> drivers/net/enic/enic.h | 1 -
> drivers/net/enic/enic_main.c | 74 ++++-------------------------------------
> drivers/net/enic/enic_res.c | 4 +-
> 3 files changed, 10 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
> index 178b94d..38b351c 100644
> --- a/drivers/net/enic/enic.h
> +++ b/drivers/net/enic/enic.h
> @@ -84,7 +84,6 @@ struct enic {
> unsigned int flags;
> unsigned int mc_count;
> unsigned int uc_count;
> - int csum_rx_enabled;
> u32 port_mtu;
> u32 rx_coalesce_usecs;
> u32 tx_coalesce_usecs;
> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
> index 9a3a027..b224551 100644
> --- a/drivers/net/enic/enic_main.c
> +++ b/drivers/net/enic/enic_main.c
> @@ -251,56 +251,6 @@ static void enic_get_ethtool_stats(struct net_device
> *netdev,
> *(data++) = ((u64 *)&vstats->rx)[enic_rx_stats[i].offset];
> }
>
> -static u32 enic_get_rx_csum(struct net_device *netdev)
> -{
> - struct enic *enic = netdev_priv(netdev);
> - return enic->csum_rx_enabled;
> -}
> -
> -static int enic_set_rx_csum(struct net_device *netdev, u32 data)
> -{
> - struct enic *enic = netdev_priv(netdev);
> -
> - if (data && !ENIC_SETTING(enic, RXCSUM))
> - return -EINVAL;
> -
> - enic->csum_rx_enabled = !!data;
> -
> - return 0;
> -}
> -
> -static int enic_set_tx_csum(struct net_device *netdev, u32 data)
> -{
> - struct enic *enic = netdev_priv(netdev);
> -
> - if (data && !ENIC_SETTING(enic, TXCSUM))
> - return -EINVAL;
> -
> - if (data)
> - netdev->features |= NETIF_F_HW_CSUM;
> - else
> - netdev->features &= ~NETIF_F_HW_CSUM;
> -
> - return 0;
> -}
> -
> -static int enic_set_tso(struct net_device *netdev, u32 data)
> -{
> - struct enic *enic = netdev_priv(netdev);
> -
> - if (data && !ENIC_SETTING(enic, TSO))
> - return -EINVAL;
> -
> - if (data)
> - netdev->features |=
> - NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN;
> - else
> - netdev->features &=
> - ~(NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN);
> -
> - return 0;
> -}
> -
> static u32 enic_get_msglevel(struct net_device *netdev)
> {
> struct enic *enic = netdev_priv(netdev);
> @@ -388,17 +338,8 @@ static const struct ethtool_ops enic_ethtool_ops = {
> .get_strings = enic_get_strings,
> .get_sset_count = enic_get_sset_count,
> .get_ethtool_stats = enic_get_ethtool_stats,
> - .get_rx_csum = enic_get_rx_csum,
> - .set_rx_csum = enic_set_rx_csum,
> - .get_tx_csum = ethtool_op_get_tx_csum,
> - .set_tx_csum = enic_set_tx_csum,
> - .get_sg = ethtool_op_get_sg,
> - .set_sg = ethtool_op_set_sg,
> - .get_tso = ethtool_op_get_tso,
> - .set_tso = enic_set_tso,
> .get_coalesce = enic_get_coalesce,
> .set_coalesce = enic_set_coalesce,
> - .get_flags = ethtool_op_get_flags,
> };
>
> static void enic_free_wq_buf(struct vnic_wq *wq, struct vnic_wq_buf *buf)
> @@ -1309,7 +1250,7 @@ static void enic_rq_indicate_buf(struct vnic_rq *rq,
> skb_put(skb, bytes_written);
> skb->protocol = eth_type_trans(skb, netdev);
>
> - if (enic->csum_rx_enabled && !csum_not_calc) {
> + if ((netdev->features & NETIF_F_RXCSUM) && !csum_not_calc) {
> skb->csum = htons(checksum);
> skb->ip_summed = CHECKSUM_COMPLETE;
> }
> @@ -2438,17 +2379,18 @@ static int __devinit enic_probe(struct pci_dev *pdev,
> dev_info(dev, "loopback tag=0x%04x\n", enic->loop_tag);
> }
> if (ENIC_SETTING(enic, TXCSUM))
> - netdev->features |= NETIF_F_SG | NETIF_F_HW_CSUM;
> + netdev->hw_features |= NETIF_F_SG | NETIF_F_HW_CSUM;
> if (ENIC_SETTING(enic, TSO))
> - netdev->features |= NETIF_F_TSO |
> + netdev->hw_features |= NETIF_F_TSO |
> NETIF_F_TSO6 | NETIF_F_TSO_ECN;
> - if (ENIC_SETTING(enic, LRO))
> - netdev->features |= NETIF_F_GRO;
> + if (ENIC_SETTING(enic, RXCSUM))
> + netdev->hw_features |= NETIF_F_RXCSUM;
> +
> + netdev->features |= netdev->hw_features;
> +
> if (using_dac)
> netdev->features |= NETIF_F_HIGHDMA;
>
> - enic->csum_rx_enabled = ENIC_SETTING(enic, RXCSUM);
> -
> err = register_netdev(netdev);
> if (err) {
> dev_err(dev, "Cannot register net device, aborting\n");
> diff --git a/drivers/net/enic/enic_res.c b/drivers/net/enic/enic_res.c
> index f111a37..6e5c635 100644
> --- a/drivers/net/enic/enic_res.c
> +++ b/drivers/net/enic/enic_res.c
> @@ -98,9 +98,9 @@ int enic_get_vnic_config(struct enic *enic)
> "vNIC MAC addr %pM wq/rq %d/%d mtu %d\n",
> enic->mac_addr, c->wq_desc_count, c->rq_desc_count, c->mtu);
> dev_info(enic_get_dev(enic), "vNIC csum tx/rx %d/%d "
> - "tso/lro %d/%d intr timer %d usec rss %d\n",
> + "tso %d intr timer %d usec rss %d\n",
> ENIC_SETTING(enic, TXCSUM), ENIC_SETTING(enic, RXCSUM),
> - ENIC_SETTING(enic, TSO), ENIC_SETTING(enic, LRO),
> + ENIC_SETTING(enic, TSO),
> c->intr_timer_usec, ENIC_SETTING(enic, RSS));
>
You are right about the driver using GRO and not LRO by default. But the
config entry from where enic gets this setting is still called LRO for
reasons out of the scope of the driver. Yes, I see that it can lead to
confusion. So, we will need to retain the ENIC_SETTING(enic, LRO) but fix
the name of that setting. Thanks for pointing this out. We will fix this
and any other changes if required and resubmit.
> return 0;
--
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
Powered by blists - more mailing lists