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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 03 Jul 2007 09:09:48 -0400 From: jamal <hadi@...erus.ca> To: Matt Carlson <mcarlson@...adcom.com> Cc: Robert Olsson <Robert.Olsson@...a.slu.se>, Evgeniy Polyakov <johnpol@....mipt.ru>, Krishna Kumar2 <krkumar2@...ibm.com>, Gagan Arneja <gaagaan@...il.com>, netdev@...r.kernel.org, Rick Jones <rick.jones2@...com>, Sridhar Samudrala <sri@...ibm.com>, David Miller <davem@...emloft.net>, Jeff Garzik <jeff@...zik.org>, Michael Chan <mchan@...adcom.com> Subject: Re: [WIP][PATCHES] Network xmit batching - tg3 support On Mon, 2007-02-07 at 14:20 -0700, Matt Carlson wrote: > > Hi Jamal. I'll be testing your patch soon, much thanks. Please let me know if you need help while doing this. What tools are you planning to test with? I have tested this patch with pktgen on a dual opteron/tg3-buggy. There is an outstanding issue in regards to clocksource - however the batch does well regardless of the clock source. > but I wanted to point out a > bug in the patch. The patch defines TG3_SKB_CB() as follows : > > #define TG3_SKB_CB(__skb) ((struct tg3_tx_cbdata *)&((__skb)->cb[0])) > > This definition will collide with the VLAN macros if TG3_VLAN_TAG_USED > is set. vlan_tx_tag_get() is defined as : > > #define vlan_tx_tag_get(__skb) (VLAN_TX_SKB_CB(__skb)->vlan_tag) > > VLAN_TX_SKB_CB is defined as : > > #define VLAN_TX_SKB_CB(__skb) \ > ((struct vlan_skb_tx_cookie *)&((__skb)->cb[0])) > yikes. Thanks for catching that - I thought i had this pretty much covered after scanning the source. So that bug exists on the e1000 as well. [It sounds very dangerous to me the way skb->cb is being used by the vlan code (i.e requires human intervention/knowledge to catch it as an issue). I had no freaking idea the vlan code was using it. Maybe a huge comment somewhere on how these cbs are being used by drivers would help or even a registration on startup to just make sure there is no conflict at a layer (i have been meaning to do the later for years now). In any case this is is a digression]. In the meantime, changing it to use byte 8 and above should do it? i.e. #define TG3_SKB_CB(__skb) ((struct tg3_tx_cbdata *)&((__skb)->cb[8])) There are 48 bytes there on the skb-cb, so there should be plenty. > Also, I think the count, max_per_txd, and nr_frags fields of the > tg3_tx_cbdata struct are not needed. Yes, you are right. That was a result of the LinuxWay(tm) (aka cutnpaste from the e1000 which needs them), Can you send me a patch for that and TG3_SKB_CB? cheers, jamal - 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