[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20100617120318.GA4059@dhcp-lab-161.englab.brq.redhat.com>
Date: Thu, 17 Jun 2010 14:03:19 +0200
From: Stanislaw Gruszka <sgruszka@...hat.com>
To: Cong Wang <amwang@...hat.com>
Cc: Ben Hutchings <bhutchings@...arflare.com>, netdev@...r.kernel.org,
herbert.xu@...hat.com, nhorman@...hat.com, davem@...emloft.net
Subject: Re: [Patch 2/2] mlx4: add dynamic LRO disable support
On Thu, Jun 17, 2010 at 06:54:28PM +0800, Cong Wang wrote:
>>> I don't think mdev->state_lock is used to protect dev->feature.
>>> rtnl_lock is. I think switching to mlx4_ethtool_op_set_flags()
>>> from the default one has already solved this.
>>
>> Ahh, you have right, may intention was use it to stop and start
>> port. Code rather should look like below:
>>
>> if (netdev_running(dev)) {
>> mutex_lock(&mdev->state_lock);
>> mlx4_en_stop_port(dev);
>> }
>>
>> dev->features ^= NETIF_F_LRO;
>>
>> if (netdev_running(dev)) {
>> rc = mlx4_en_start_port(dev);
>> mutex_unlock(&mdev->state_lock);
>> if (rc)
>> en_err(priv, "Failed to restart port\n");
>> }
>>
>
> Hmm, you mean ->features should be changed after port is stopped?
Actually not ->features variable, but NETIF_F_LRO bit, as only this
bit is used in rx path.
> Why?
For reasons you talked before in this thread :) to do not change
LRO in the middle of receiving packages.
Stanislaw
--
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