netdev - Re: [net-next 04/14] net: Fix issue with netdev_tx_reset_queue not resetting queue from XOFF state

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: <CAKgT0UcZZNOH=D5mJDaw3_6femW120ZG0cBc8Nz=-=8h6X17kg@mail.gmail.com>
Date:	Fri, 20 Apr 2012 20:24:33 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	Tom Herbert <therbert@...gle.com>
Cc:	Alexander Duyck <alexander.h.duyck@...el.com>,
	Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
	davem@...emloft.net, netdev@...r.kernel.org, gospo@...hat.com,
	sassmann@...hat.com, John Fastabend <john.r.fastabend@...el.com>
Subject: Re: [net-next 04/14] net: Fix issue with netdev_tx_reset_queue not
 resetting queue from XOFF state

On Fri, Apr 20, 2012 at 2:31 PM, John Fastabend
<john.r.fastabend@...el.com> wrote:
> On 4/20/2012 2:19 PM, Tom Herbert wrote:
>> Hi Jeff,
>>
>>> @@ -3244,7 +3246,6 @@ static void igb_clean_tx_ring(struct igb_ring *tx_ring)
>>>                buffer_info = &tx_ring->tx_buffer_info[i];
>>>                igb_unmap_and_free_tx_resource(tx_ring, buffer_info);
>>>        }
>>> -       netdev_tx_reset_queue(txring_txq(tx_ring));
>>>
>> Why is it necessary to remove this?  If rings are being freed with
>> going through completion path then we would need this reset.  Is this
>> being done elsewhere maybe?
>>
>
> Alex moved this into the igb_configure_tx_ring() iirc to catch an ethtool
> test case. He assured me when I reviewed the patch that igb_configure_tx_ring()
> would always be called before netif_carrier_on() so I think(?) that should
> be OK.
>
> The above removed case was called after netif_carrier_off() anyways.

I don't recall the exact reason now, as John said I think it had to do
with the ethtool tests not using the same cleanup routine and leaving
us in a bad state.  I am pretty sure there was some path in which
where the call was didn't work but I do not recall the exact details
now.  Most of the reason for moving it is due to the fact that the
reset is now also clearing the bit, and from the driver perspective we
didn't need it in two places.  After looking it all over again, I
suppose this causes a cosmetic issue for the bql "inflight" statistic
in sysfs since the value will be retained until the interface is
brought back up with this change.  Is that an issue or something that
can be lived with since the interface isn't active anyway in this
case?

Thanks,

Alex
--
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