netdev - Re: [PATCH] ucc_geth: Add 8 bytes to max TX frame for VLANs

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] [day] [month] [year] [list]

Date:	Tue, 11 Nov 2008 15:40:01 -0600
From:	"Andy Fleming" <afleming@...il.com>
To:	"Joakim Tjernlund" <joakim.tjernlund@...nsmode.se>
Cc:	"Jeff Garzik" <jeff@...zik.org>,
	"Andy Fleming" <afleming@...escale.com>, netdev@...r.kernel.org
Subject: Re: [PATCH] ucc_geth: Add 8 bytes to max TX frame for VLANs

On Fri, Nov 7, 2008 at 4:18 AM, Joakim Tjernlund
<joakim.tjernlund@...nsmode.se> wrote:
> On Sun, 2008-11-02 at 08:18 -0500, Jeff Garzik wrote:
>> Andy Fleming wrote:
>> > From: Joakim Tjernlund <Joakim.Tjernlund@...nsmode.se>
>> >
>> > Creating a VLAN interface on top of ucc_geth adds 4 bytes
>> > to the frame and the HW controller is not prepared to
>> > TX a frame bigger than 1518 bytes which is 4 bytes too
>> > small for a full VLAN frame. Also add 4 extra bytes for future
>> > expansion.
>> >
>> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@...nsmode.se>
>> > ---
>> >  drivers/net/ucc_geth.c |    6 +++---
>> >  1 files changed, 3 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
>> > index 8f944e5..8fed4bb 100644
>> > --- a/drivers/net/ucc_geth.c
>> > +++ b/drivers/net/ucc_geth.c
>> > @@ -113,10 +113,10 @@ static struct ucc_geth_info ugeth_primary_info = {
>> >     .maxGroupAddrInHash = 4,
>> >     .maxIndAddrInHash = 4,
>> >     .prel = 7,
>> > -   .maxFrameLength = 1518,
>> > +   .maxFrameLength = 1518+8, /* Add 4 bytes for VLAN tags and 4 extra bytes */
>> >     .minFrameLength = 64,
>> > -   .maxD1Length = 1520,
>> > -   .maxD2Length = 1520,
>> > +   .maxD1Length = 1520+8,
>> > +   .maxD2Length = 1520+8,
>> >     .vlantype = 0x8100,
>>
>> Is this still needed?
>>
>> I found this buried in my inbox.
>>
>> If so, please resend with the "+8" converted into a named constant.
>> Hardcoding lengths like this without named constants (or comments, on
>> the other lines) tends to be an error-prone practice.
>
> Yes, it is. I do recall the maintainer had some comments though.
> Secondly, I think it should be changed to 8 extra bytes so
> you can fit VLAN+pppoe=4+8 bytes. Possibly it could
> be configuarble at complile time.
>
> Andy, you think you can work out the details with the maintainer?
> I am much to busy ATM.


Hmm... me, too.  I'm not really familiar with this driver.  I suspect
that all of the stuff in that structure should be yanked out, and done
more sensibly, but I don't have time to do it.  I'll ping him about
it.

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