Re: [PATCH][TG3]Some cleanups
From: jamal [EMAIL PROTECTED] Date: Sun, 07 Oct 2007 11:12:21 -0400 Ok, attached patch against net-2.6.24 from this morning. I am setting up some equipment for testing as i type this - so i will test for any regressions. If you dont hear from me on the subject then all went ok. This cleanup only makes sense if we go with your TX batching interfaces. They make the TX batching support patch for this driver nice and clean, but it makes zero sense in any other context. In fact, it adds more memory references in the TX pacth, and in fact does so by adding usage of the skb-cb[] which the driver didn't need to do previously. So I'm going to hold off on this one for now, keep it in your TX batching changes instead. THanks. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][TG3]Some cleanups
On Sun, 2007-07-10 at 23:32 -0700, David Miller wrote: This cleanup only makes sense if we go with your TX batching interfaces. They make the TX batching support patch for this driver nice and clean, but it makes zero sense in any other context. In fact, it adds more memory references in the TX pacth, and in fact does so by adding usage of the skb-cb[] which the driver didn't need to do previously. So I'm going to hold off on this one for now, keep it in your TX batching changes instead. The batching benefits from it because it reuses code. But i would put readability as something of no value. In any case i would defer this for later. cheers, jamal - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][TG3]Some cleanups
On Wed, 2007-03-10 at 09:18 -0400, jamal wrote:
On Tue, 2007-02-10 at 16:33 -0700, Michael Chan wrote:
Seems ok to me. I think we should make it more clear that we're
skipping over the VLAN tag:
(struct tg3_tx_cbdata *)((__skb)-cb[sizeof(struct vlan_skb_tx_cookie)])
Will do - thanks Michael.
Ok, attached patch against net-2.6.24 from this morning. I am setting up
some equipment for testing as i type this - so i will test for any
regressions. If you dont hear from me on the subject then all went ok.
cheers,
jamal
[TG3] Some cleanups
Make the xmit path code better functionally organized and more readable.
Matt Carlson contributed the moving of the VLAN formatting into
_prep_frame() portion and Michael Chan eyeballed the patch and
made it better.
Signed-off-by: Jamal Hadi Salim [EMAIL PROTECTED]
---
commit 190674ff1fe0b7bddf038c2bfddf45b9c6418e2a
tree d6f03d10d94a39ad22fd590b30c6d50c81f3eb5c
parent 0a14acec89454d350c1bd141b3309df421433f3c
author Jamal Hadi Salim [EMAIL PROTECTED] Sun, 07 Oct 2007 08:10:32 -0400
committer Jamal Hadi Salim [EMAIL PROTECTED] Sun, 07 Oct 2007 08:10:32 -0400
drivers/net/tg3.c | 281 -
1 files changed, 172 insertions(+), 109 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index d4ac6e9..ce6f02f 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -3910,47 +3910,72 @@ static void tg3_set_txd(struct tg3 *tp, int entry,
txd-vlan_tag = vlan_tag TXD_VLAN_TAG_SHIFT;
}
-/* hard_start_xmit for devices that don't have any bugs and
- * support TG3_FLG2_HW_TSO_2 only.
- */
-static int tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
+struct tg3_tx_cbdata {
+ u32 base_flags;
+ unsigned int mss;
+};
+/* using cb[8] skips over the VLAN tag:
+ * (struct tg3_tx_cbdata *)((__skb)-cb[sizeof(struct vlan_skb_tx_cookie)])
+*/
+#define TG3_SKB_CB(__skb) ((struct tg3_tx_cbdata *)((__skb)-cb[8]))
+#define NETDEV_TX_DROPPED -5
+
+static int tg3_prep_bug_frame(struct sk_buff *skb, struct net_device *dev)
{
+ struct tg3_tx_cbdata *cb = TG3_SKB_CB(skb);
+#if TG3_VLAN_TAG_USED
struct tg3 *tp = netdev_priv(dev);
- dma_addr_t mapping;
- u32 len, entry, base_flags, mss;
+ u32 vlantag = 0;
- len = skb_headlen(skb);
+ if (tp-vlgrp != NULL vlan_tx_tag_present(skb))
+ vlantag = (TXD_FLAG_VLAN | (vlan_tx_tag_get(skb) 16));
- /* We are running in BH disabled context with netif_tx_lock
-* and TX reclaim runs via tp-napi.poll inside of a software
-* interrupt. Furthermore, IRQ processing runs lockless so we have
-* no IRQ context deadlocks to worry about either. Rejoice!
-*/
- if (unlikely(tg3_tx_avail(tp) = (skb_shinfo(skb)-nr_frags + 1))) {
- if (!netif_queue_stopped(dev)) {
- netif_stop_queue(dev);
+ cb-base_flags = vlantag;
+#endif
- /* This is a hard error, log it. */
- printk(KERN_ERR PFX %s: BUG! Tx Ring full when
- queue awake!\n, dev-name);
+ cb-mss = skb_shinfo(skb)-gso_size;
+ if (cb-mss != 0) {
+ if (skb_header_cloned(skb)
+ pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
+ dev_kfree_skb(skb);
+ return NETDEV_TX_DROPPED;
}
- return NETDEV_TX_BUSY;
+
+ cb-base_flags |= (TXD_FLAG_CPU_PRE_DMA |
+ TXD_FLAG_CPU_POST_DMA);
}
- entry = tp-tx_prod;
- base_flags = 0;
- mss = 0;
- if ((mss = skb_shinfo(skb)-gso_size) != 0) {
+ if (skb-ip_summed == CHECKSUM_PARTIAL)
+ cb-base_flags |= TXD_FLAG_TCPUDP_CSUM;
+
+ return NETDEV_TX_OK;
+}
+
+static int tg3_prep_frame(struct sk_buff *skb, struct net_device *dev)
+{
+ struct tg3_tx_cbdata *cb = TG3_SKB_CB(skb);
+#if TG3_VLAN_TAG_USED
+ struct tg3 *tp = netdev_priv(dev);
+ u32 vlantag = 0;
+
+ if (tp-vlgrp != NULL vlan_tx_tag_present(skb))
+ vlantag = (TXD_FLAG_VLAN | (vlan_tx_tag_get(skb) 16));
+
+ cb-base_flags = vlantag;
+#endif
+
+ cb-mss = skb_shinfo(skb)-gso_size;
+ if (cb-mss != 0) {
int tcp_opt_len, ip_tcp_len;
if (skb_header_cloned(skb)
pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
dev_kfree_skb(skb);
- goto out_unlock;
+ return NETDEV_TX_DROPPED;
}
if (skb_shinfo(skb)-gso_type SKB_GSO_TCPV6)
- mss |= (skb_headlen(skb) - ETH_HLEN) 9;
+ cb-mss |= (skb_headlen(skb) - ETH_HLEN) 9;
else {
struct iphdr *iph = ip_hdr(skb);
@@ -3958,32 +3983,58 @@ static int tg3_start_xmit(struct sk_buff *skb,
Re: [PATCH][TG3]Some cleanups
On Tue, 2007-02-10 at 16:33 -0700, Michael Chan wrote: Seems ok to me. I think we should make it more clear that we're skipping over the VLAN tag: (struct tg3_tx_cbdata *)((__skb)-cb[sizeof(struct vlan_skb_tx_cookie)]) Will do - thanks Michael. cheers, jamal - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][TG3]Some cleanups
On Mon, 2007-01-10 at 17:21 -0700, Michael Chan wrote: Jamal, in tg3_enqueue_buggy(), we may have to call tg3_tso_bug() which will recursively call tg3_start_xmit_dma_bug() after segmenting the TSO packet into normal packets. We need to restore the VLAN tag so that the GSO code will create the chain of segmented SKBs with the proper VLAN tag. Excellent eye Michael. The simplest solution seems to me to modify the definition of TG3_SKB_CB as i did for e1000 from: (struct tg3_tx_cbdata *)((__skb)-cb[0]) to: (struct tg3_tx_cbdata *)((__skb)-cb[8]) that way the vlan tags are always present and no need to recreate them. What do you think? cheers, jamal - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][TG3]Some cleanups
On Tue, 2007-10-02 at 08:37 -0400, jamal wrote: The simplest solution seems to me to modify the definition of TG3_SKB_CB as i did for e1000 from: (struct tg3_tx_cbdata *)((__skb)-cb[0]) to: (struct tg3_tx_cbdata *)((__skb)-cb[8]) that way the vlan tags are always present and no need to recreate them. What do you think? Seems ok to me. I think we should make it more clear that we're skipping over the VLAN tag: (struct tg3_tx_cbdata *)((__skb)-cb[sizeof(struct vlan_skb_tx_cookie)]) - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][TG3]Some cleanups
On Sun, 2007-09-30 at 14:11 -0400, jamal wrote: Here are some non-batching related changes that i have in my batching tree. Like the e1000e, they make the xmit code more readable. I wouldnt mind if you take them over. Jamal, in tg3_enqueue_buggy(), we may have to call tg3_tso_bug() which will recursively call tg3_start_xmit_dma_bug() after segmenting the TSO packet into normal packets. We need to restore the VLAN tag so that the GSO code will create the chain of segmented SKBs with the proper VLAN tag. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][TG3]Some cleanups
Here are some non-batching related changes that i have in my batching
tree. Like the e1000e, they make the xmit code more readable.
I wouldnt mind if you take them over.
cheers,
jamal
[TG3] Some cleanups
These cleanups make the xmit path code better functionally organized.
Matt Carlson contributed the moving of the VLAN formatting into
_prep_frame() portion.
Signed-off-by: Jamal Hadi Salim [EMAIL PROTECTED]
---
commit 260dbcc4b0195897c539c5ff79d95afdddeb3378
tree b2047b0e474abb9f05dd40c22af7f0a86369957d
parent ad63c288ce980907f68d94d5faac08625c0b1782
author Jamal Hadi Salim [EMAIL PROTECTED] Sun, 30 Sep 2007 14:01:46 -0400
committer Jamal Hadi Salim [EMAIL PROTECTED] Sun, 30 Sep 2007 14:01:46 -0400
drivers/net/tg3.c | 278 -
1 files changed, 169 insertions(+), 109 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index d4ac6e9..5a864bd 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -3910,47 +3910,69 @@ static void tg3_set_txd(struct tg3 *tp, int entry,
txd-vlan_tag = vlan_tag TXD_VLAN_TAG_SHIFT;
}
-/* hard_start_xmit for devices that don't have any bugs and
- * support TG3_FLG2_HW_TSO_2 only.
- */
-static int tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
+struct tg3_tx_cbdata {
+ u32 base_flags;
+ unsigned int mss;
+};
+#define TG3_SKB_CB(__skb) ((struct tg3_tx_cbdata *)((__skb)-cb[0]))
+#define NETDEV_TX_DROPPED -5
+
+static int tg3_prep_bug_frame(struct sk_buff *skb, struct net_device *dev)
{
+ struct tg3_tx_cbdata *cb = TG3_SKB_CB(skb);
+#if TG3_VLAN_TAG_USED
struct tg3 *tp = netdev_priv(dev);
- dma_addr_t mapping;
- u32 len, entry, base_flags, mss;
+ u32 vlantag = 0;
- len = skb_headlen(skb);
+ if (tp-vlgrp != NULL vlan_tx_tag_present(skb))
+ vlantag = (TXD_FLAG_VLAN | (vlan_tx_tag_get(skb) 16));
- /* We are running in BH disabled context with netif_tx_lock
-* and TX reclaim runs via tp-napi.poll inside of a software
-* interrupt. Furthermore, IRQ processing runs lockless so we have
-* no IRQ context deadlocks to worry about either. Rejoice!
-*/
- if (unlikely(tg3_tx_avail(tp) = (skb_shinfo(skb)-nr_frags + 1))) {
- if (!netif_queue_stopped(dev)) {
- netif_stop_queue(dev);
+ cb-base_flags = vlantag;
+#endif
- /* This is a hard error, log it. */
- printk(KERN_ERR PFX %s: BUG! Tx Ring full when
- queue awake!\n, dev-name);
+ cb-mss = skb_shinfo(skb)-gso_size;
+ if (cb-mss != 0) {
+ if (skb_header_cloned(skb)
+ pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
+ dev_kfree_skb(skb);
+ return NETDEV_TX_DROPPED;
}
- return NETDEV_TX_BUSY;
+
+ cb-base_flags |= (TXD_FLAG_CPU_PRE_DMA |
+ TXD_FLAG_CPU_POST_DMA);
}
- entry = tp-tx_prod;
- base_flags = 0;
- mss = 0;
- if ((mss = skb_shinfo(skb)-gso_size) != 0) {
+ if (skb-ip_summed == CHECKSUM_PARTIAL)
+ cb-base_flags |= TXD_FLAG_TCPUDP_CSUM;
+
+ return NETDEV_TX_OK;
+}
+
+static int tg3_prep_frame(struct sk_buff *skb, struct net_device *dev)
+{
+ struct tg3_tx_cbdata *cb = TG3_SKB_CB(skb);
+#if TG3_VLAN_TAG_USED
+ struct tg3 *tp = netdev_priv(dev);
+ u32 vlantag = 0;
+
+ if (tp-vlgrp != NULL vlan_tx_tag_present(skb))
+ vlantag = (TXD_FLAG_VLAN | (vlan_tx_tag_get(skb) 16));
+
+ cb-base_flags = vlantag;
+#endif
+
+ cb-mss = skb_shinfo(skb)-gso_size;
+ if (cb-mss != 0) {
int tcp_opt_len, ip_tcp_len;
if (skb_header_cloned(skb)
pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
dev_kfree_skb(skb);
- goto out_unlock;
+ return NETDEV_TX_DROPPED;
}
if (skb_shinfo(skb)-gso_type SKB_GSO_TCPV6)
- mss |= (skb_headlen(skb) - ETH_HLEN) 9;
+ cb-mss |= (skb_headlen(skb) - ETH_HLEN) 9;
else {
struct iphdr *iph = ip_hdr(skb);
@@ -3958,32 +3980,58 @@ static int tg3_start_xmit(struct sk_buff *skb, struct
net_device *dev)
ip_tcp_len = ip_hdrlen(skb) + sizeof(struct tcphdr);
iph-check = 0;
- iph-tot_len = htons(mss + ip_tcp_len + tcp_opt_len);
- mss |= (ip_tcp_len + tcp_opt_len) 9;
+ iph-tot_len = htons(cb-mss + ip_tcp_len
++ tcp_opt_len);
+ cb-mss |= (ip_tcp_len + tcp_opt_len) 9;
}
- base_flags |=
Re: [PATCH][TG3]Some cleanups
On Sun, 2007-30-09 at 14:11 -0400, jamal wrote: Here are some non-batching related changes that i have in my batching tree. Like the e1000e, they make the xmit code more readable. I wouldnt mind if you take them over. Should have mentioned: Against Dave's tree from a few hours back. cheers, jamal - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
