* [ofa-general] Re: [PATCH 00/10] Implement batching skb API
From: Krishna Kumar2 @ 2007-07-20 7:47 UTC (permalink / raw)
To: Stephen Hemminger
Cc: jagana, johnpol, herbert, gaagaan, Robert.Olsson, kumarkr,
mcarlson, peter.p.waskiewicz.jr, hadi, kaber, jeff, general,
mchan, tgraf, netdev, sri, davem, rdreier
In-Reply-To: <20070720081848.7cc652fb@oldman>
Stephen Hemminger <shemminger@linux-foundation.org> wrote on 07/20/2007
12:48:48 PM:
> You may see worse performance with batching in the real world when
> running over WAN's. Like TSO, batching will generate back to back packet
> trains that are subject to multi-packet synchronized loss. The problem is
that
> intermediate router queues are often close to full, and when a long
string
> of packets arrives back to back only the first ones will get in, the rest
> get dropped. Normal sends have at least minimal pacing so they are less
> likely do get synchronized drop.
Also forgot to mention in the previous mail, if performance is seen to be
dipping,
batching can be disabled on WAN's by:
echo 0 > /sys/class/net/<dev>/tx_batch_skbs
and use batching on local/site networks in that case.
^ permalink raw reply
* Re: [PATCH] [updated] PHY fixed driver: rework release path and update phy_id notation
From: Vitaly Bordug @ 2007-07-20 7:50 UTC (permalink / raw)
To: Andrew Morton; +Cc: linuxppc-dev, linux-kernel, Jeff Garzik, netdev
In-Reply-To: <20070719232337.184942e0.akpm@linux-foundation.org>
On Thu, 19 Jul 2007 23:23:37 -0700
Andrew Morton wrote:
> On Thu, 19 Jul 2007 03:38:04 +0400 Vitaly Bordug
> <vitb@kernel.crashing.org> wrote:
>
> >
> > device_bind_driver() error code returning has been fixed.
> > release() function has been written, so that to free resources
> > in correct way; the release path is now clean.
> >
> > Before the rework, it used to cause
> > Device 'fixed@100:1' does not have a release() function, it is
> > broken and must be fixed.
> > BUG: at drivers/base/core.c:104 device_release()
> >
> > Call Trace:
> > [<ffffffff802ec380>] kobject_cleanup+0x53/0x7e
> > [<ffffffff802ec3ab>] kobject_release+0x0/0x9
> > [<ffffffff802ecf3f>] kref_put+0x74/0x81
> > [<ffffffff8035493b>] fixed_mdio_register_device+0x230/0x265
> > [<ffffffff80564d31>] fixed_init+0x1f/0x35
> > [<ffffffff802071a4>] init+0x147/0x2fb
> > [<ffffffff80223b6e>] schedule_tail+0x36/0x92
> > [<ffffffff8020a678>] child_rip+0xa/0x12
> > [<ffffffff80311714>] acpi_ds_init_one_object+0x0/0x83
> > [<ffffffff8020705d>] init+0x0/0x2fb
> > [<ffffffff8020a66e>] child_rip+0x0/0x12
> >
> >
> > Also changed the notation of the fixed phy definition on
> > mdio bus to the form of <speed>+<duplex> to make it able to be used
> > by gianfar and ucc_geth that define phy_id strictly as "%d:%d" and
> > cleaned up the whitespace issues.
> >
>
> Confused. Does the above refer to the difference between this patch
> and the previous version, or does it just describe this patch?
> Hopefully the latter, because the former isn't interesting, long-term.
>
Latter. IOW, that does mean, that mdio bus registered by this driver, now uses
same naming conventioun that other PHYLIB things use. Hereby it will make it able to be used in
NIC drivers other than fs_enet (and gianfar and ucc_geth are now points of interest).
> If is _is_ a full standalone description of this patch then it's a
> bit hard to follow ;)
>
Hmm -so what are my options - change the description and resubmit?
> > +config FIXED_MII_1000_FDX
> > + bool "Emulation for 1000M Fdx fixed PHY behavior"
> > + depends on FIXED_PHY
> > +
> > +config FIXED_MII_AMNT
> > + int "Number of emulated PHYs to allocate "
> > + depends on FIXED_PHY
> > + default "1"
> > + ---help---
> > + Sometimes it is required to have several independent
> > emulated
> > + PHYs on the bus (in case of multi-eth but phy-less HW for
> > instance).
> > + This control will have specified number allocated for each
> > fixed
> > + PHY type enabled.
>
> Shouldn't these be runtime options (ie: module parameters)?
>
I thought about it but this thing is more like the one that will never tend/required to change while\
configured.. Will add if you see it appropriate though.
> >
> > ...
> >
> > + * Private information hoder for mii_bus
>
> tpyo.
ok
--
Sincerely, Vitaly
^ permalink raw reply
* Re: [PATCH] [updated] PHY fixed driver: rework release path and update phy_id notation
From: Andrew Morton @ 2007-07-20 7:57 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: Jeff Garzik, linuxppc-dev, netdev, linux-kernel
In-Reply-To: <20070720115039.05f50f0f@localhost.localdomain>
On Fri, 20 Jul 2007 11:50:39 +0400 Vitaly Bordug <vitb@kernel.crashing.org> wrote:
> On Thu, 19 Jul 2007 23:23:37 -0700
> Andrew Morton wrote:
>
> > On Thu, 19 Jul 2007 03:38:04 +0400 Vitaly Bordug
> > <vitb@kernel.crashing.org> wrote:
> >
> > >
> > > device_bind_driver() error code returning has been fixed.
> > > release() function has been written, so that to free resources
> > > in correct way; the release path is now clean.
> > >
> > > Before the rework, it used to cause
> > > Device 'fixed@100:1' does not have a release() function, it is
> > > broken and must be fixed.
> > > BUG: at drivers/base/core.c:104 device_release()
> > >
> > > Call Trace:
> > > [<ffffffff802ec380>] kobject_cleanup+0x53/0x7e
> > > [<ffffffff802ec3ab>] kobject_release+0x0/0x9
> > > [<ffffffff802ecf3f>] kref_put+0x74/0x81
> > > [<ffffffff8035493b>] fixed_mdio_register_device+0x230/0x265
> > > [<ffffffff80564d31>] fixed_init+0x1f/0x35
> > > [<ffffffff802071a4>] init+0x147/0x2fb
> > > [<ffffffff80223b6e>] schedule_tail+0x36/0x92
> > > [<ffffffff8020a678>] child_rip+0xa/0x12
> > > [<ffffffff80311714>] acpi_ds_init_one_object+0x0/0x83
> > > [<ffffffff8020705d>] init+0x0/0x2fb
> > > [<ffffffff8020a66e>] child_rip+0x0/0x12
> > >
> > >
> > > Also changed the notation of the fixed phy definition on
> > > mdio bus to the form of <speed>+<duplex> to make it able to be used
> > > by gianfar and ucc_geth that define phy_id strictly as "%d:%d" and
> > > cleaned up the whitespace issues.
> > >
> >
> > Confused. Does the above refer to the difference between this patch
> > and the previous version, or does it just describe this patch?
> > Hopefully the latter, because the former isn't interesting, long-term.
> >
> Latter. IOW, that does mean, that mdio bus registered by this driver, now uses
> same naming conventioun that other PHYLIB things use. Hereby it will make it able to be used in
> NIC drivers other than fs_enet (and gianfar and ucc_geth are now points of interest).
>
> > If is _is_ a full standalone description of this patch then it's a
> > bit hard to follow ;)
> >
> Hmm -so what are my options - change the description and resubmit?
umm, I guess it's OK as-is. But it wasn't clear to me which sort of
changelog it was.
> > > +config FIXED_MII_1000_FDX
> > > + bool "Emulation for 1000M Fdx fixed PHY behavior"
> > > + depends on FIXED_PHY
> > > +
> > > +config FIXED_MII_AMNT
> > > + int "Number of emulated PHYs to allocate "
> > > + depends on FIXED_PHY
> > > + default "1"
> > > + ---help---
> > > + Sometimes it is required to have several independent
> > > emulated
> > > + PHYs on the bus (in case of multi-eth but phy-less HW for
> > > instance).
> > > + This control will have specified number allocated for each
> > > fixed
> > > + PHY type enabled.
> >
> > Shouldn't these be runtime options (ie: module parameters)?
> >
> I thought about it but this thing is more like the one that will never tend/required to change while\
> configured.. Will add if you see it appropriate though.
99% of users don't compile their own kernels: their vendor will have to
make this decision for them, and it sounds like any decision which they
make will be wrong for some of their users?
^ permalink raw reply
* [ofa-general] Re: [PATCH 00/10] Implement batching skb API
From: Stephen Hemminger @ 2007-07-20 7:57 UTC (permalink / raw)
To: Krishna Kumar2
Cc: jagana, johnpol, herbert, gaagaan, Robert.Olsson, kumarkr,
mcarlson, peter.p.waskiewicz.jr, hadi, kaber, jeff, general,
mchan, tgraf, netdev, sri, davem, rdreier
In-Reply-To: <OF37602E5B.BA36F154-ON6525731E.0028A4E9-6525731E.00293CC3@in.ibm.com>
On Fri, 20 Jul 2007 13:00:25 +0530
Krishna Kumar2 <krkumar2@in.ibm.com> wrote:
> Stephen Hemminger <shemminger@linux-foundation.org> wrote on 07/20/2007
> 12:48:48 PM:
>
> > You may see worse performance with batching in the real world when
> > running over WAN's. Like TSO, batching will generate back to back packet
> > trains that are subject to multi-packet synchronized loss. The problem is
> that
> > intermediate router queues are often close to full, and when a long
> string
> > of packets arrives back to back only the first ones will get in, the rest
> > get dropped. Normal sends have at least minimal pacing so they are less
> > likely do get synchronized drop.
>
> Hi Stephen,
>
> OK. The difference that I could see is that in existing code, the "minimal
> pacing" also could lead to (possibly slighly lesser) loss since sends are
> quick iterations at the IP layer, while in batching sends are iterative at
> the driver layer.
>
> Is it an issue ? Any suggestions ?
Not an immediate issue, but it is the kind of thing that could cause performance
regression reports if it was used on every interface by default.
^ permalink raw reply
* [PATCH 0/10] ps3: fixes for ps3_gelic driver
From: Masakazu Mokuno @ 2007-07-20 8:08 UTC (permalink / raw)
To: Jeff Garzik; +Cc: cbe-oss-dev, netdev
This patchset are fixes and updates incorporating the comments from Jeff
Garzik and Stephen Hemminger.
Please accept the following patches for the ps3_gelic driver. Thanks.
[1] ps3: fix wrong calculation of rx descriptor address
[2] ps3: some minor cleanups
[3] ps3: tx descriptor handling cleanup
[4] ps3: removed defines no longer used
[5] ps3: removed conditional ethtool support
[6] ps3: use net_device_stats of net_device structure
[7] ps3: use ethX as the name of irq
[8] ps3: removed calling netif_poll_enable() in open()
[9] ps3: fix rare issue that reenabling rx DMA fails
[10] ps3: reduce allocation size of rx skb buffers
--
Masakazu MOKUNO
^ permalink raw reply
* Re: Possible eHEA performance issue
From: Thomas Klein @ 2007-07-20 8:17 UTC (permalink / raw)
To: Michael Neuling
Cc: Thomas Klein, Jan-Bernd Themann, netdev, Christoph Raisch,
Stefan Roscher, linux-ppc, anton
In-Reply-To: <4634.1184900524@neuling.org>
Michael Neuling wrote:
> From ehea_start_xmit in ehea_main.c we have:
>
> if (unlikely(atomic_read(&pr->swqe_avail) <= 1)) {
> spin_lock_irqsave(&pr->netif_queue, flags);
> if (unlikely(atomic_read(&pr->swqe_avail) <= 1)) {
> pr->p_stats.queue_stopped++;
> netif_stop_queue(dev);
> pr->queue_stopped = 1;
> }
> spin_unlock_irqrestore(&pr->netif_queue, flags);
> }
>
> Since the conditions are the same, isn't it likely that the second 'if'
> is going to be taken. Hence, shouldn't the second 'unlikely' hint be
> removed or even changed to likely?
>
> Either way, some documentation here as to why it's done this way would
> be useful. I assume the atomic_read is cheap compared to the
> spin_unlock_irqsave, so we quickly check swqe_avail before we check it
> again properly with the lock on so we can change some stuff.
>
> Mikey
Hi Mike,
good point the second if could be a likely(). The impact isn't that big
because the if statement is true in the unlikely() case that the send queue
is full - which doesn't happen often. Anyway we will modify this in one of
the next driver versions. Thanks for the hint!
Thomas
^ permalink raw reply
* [PATCH] ucc_geth_mii: fix __exit called from __init
From: Domen Puncer @ 2007-07-20 8:19 UTC (permalink / raw)
To: leoli; +Cc: netdev, linuxppc-embedded
void __exit uec_mdio_exit(void) is called from
- static int __init ucc_geth_init(void)
- static void __exit ucc_geth_exit(void)
First one would make error path more than just an error.
Signed-off-by: Domen Puncer <domen.puncer@telargo.com>
---
drivers/net/ucc_geth_mii.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: work-powerpc.git/drivers/net/ucc_geth_mii.c
===================================================================
--- work-powerpc.git.orig/drivers/net/ucc_geth_mii.c
+++ work-powerpc.git/drivers/net/ucc_geth_mii.c
@@ -272,7 +272,8 @@ int __init uec_mdio_init(void)
return of_register_platform_driver(&uec_mdio_driver);
}
-void __exit uec_mdio_exit(void)
+/* called from __init ucc_geth_init, therefore can not be __exit */
+void uec_mdio_exit(void)
{
of_unregister_platform_driver(&uec_mdio_driver);
}
^ permalink raw reply
* [PATCH 1/10] ps3: fix wrong calculation of rx descriptor address
From: Masakazu Mokuno @ 2007-07-20 8:20 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, cbe-oss-dev
Fixed the bug that calculation of the address of rx descriptor was
wrong.
Signed-off-by: Masakazu Mokuno <mokuno@sm.sony.co.jp>
---
drivers/net/ps3_gelic_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/net/ps3_gelic_net.c
+++ b/drivers/net/ps3_gelic_net.c
@@ -1107,7 +1107,7 @@ static int gelic_net_open(struct net_dev
card->descr, GELIC_NET_TX_DESCRIPTORS))
goto alloc_tx_failed;
if (gelic_net_init_chain(card, &card->rx_chain,
- card->descr + GELIC_NET_RX_DESCRIPTORS,
+ card->descr + GELIC_NET_TX_DESCRIPTORS,
GELIC_NET_RX_DESCRIPTORS))
goto alloc_rx_failed;
--
Masakazu MOKUNO
^ permalink raw reply
* Re: ANNOUNCE: igb: Intel 82575 Gigabit Ethernet driver (PCI-Express)
From: Stefan Rompf @ 2007-07-20 8:23 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Kok, Auke, Jeff Garzik, NetDev, Arjan van de Ven, Ronciak, John,
Mitch Williams
In-Reply-To: <20070720073347.GA29575@infradead.org>
Am Freitag, 20. Juli 2007 09:33 schrieben Sie:
> Well, these useless abstractions are really annoying and nothing we allow
> into other drivers either.
However, they make it easier for multiple developers to work as a team. This
is useful for development pace and makes maintainance less dependant on one
person.
Stefan
^ permalink raw reply
* [PATCH 2/10] ps3: some minor cleanups
From: Masakazu Mokuno @ 2007-07-20 8:22 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, cbe-oss-dev
- Removed the embarrassing definition which was used in only one
place.
- Fixed wrong initialization of dmac_cmd_status.
Signed-off-by: Masakazu Mokuno <mokuno@sm.sony.co.jp>
---
drivers/net/ps3_gelic_net.c | 5 +++--
drivers/net/ps3_gelic_net.h | 1 -
2 files changed, 3 insertions(+), 3 deletions(-)
--- a/drivers/net/ps3_gelic_net.c
+++ b/drivers/net/ps3_gelic_net.c
@@ -290,7 +290,8 @@ static void gelic_net_release_rx_chain(s
descr->buf_addr = 0;
dev_kfree_skb_any(descr->skb);
descr->skb = NULL;
- descr->dmac_cmd_status = GELIC_NET_DESCR_NOT_IN_USE;
+ gelic_net_set_descr_status(descr,
+ GELIC_NET_DESCR_NOT_IN_USE);
}
descr = descr->next;
} while (descr != card->rx_chain.head);
@@ -374,7 +375,7 @@ static void gelic_net_release_tx_descr(s
descr->skb = NULL;
/* set descr status */
- descr->dmac_cmd_status = GELIC_NET_DMAC_CMDSTAT_NOT_IN_USE;
+ gelic_net_set_descr_status(descr, GELIC_NET_DESCR_NOT_IN_USE);
}
/**
--- a/drivers/net/ps3_gelic_net.h
+++ b/drivers/net/ps3_gelic_net.h
@@ -133,7 +133,6 @@ enum gelic_net_int1_status {
* interrupt status */
#define GELIC_NET_DMAC_CMDSTAT_CHAIN_END 0x00000002 /* RXDCEIS:DMA stopped */
-#define GELIC_NET_DMAC_CMDSTAT_NOT_IN_USE 0xb0000000
#define GELIC_NET_DESCR_IND_PROC_SHIFT 28
#define GELIC_NET_DESCR_IND_PROC_MASKO 0x0fffffff
--
Masakazu MOKUNO
^ permalink raw reply
* [PATCH 3/10] ps3: tx descriptor handling cleanup
From: Masakazu Mokuno @ 2007-07-20 8:24 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, cbe-oss-dev
gelic: TX descriptor handling cleanup
- Emitted return value of NETDEV_TX_LOCKED when DMA map or kick
failure.
Now it would free the skb, update drop packet statistics
and return OK. Requested from Jeff Garzik.
- Enable tx queue if number of free descriptors are more than 2
- Fixed descriptor leak if dma map for second descriptor failed
- Stopped calling xmit handler from interrupt handler in order
to recheck tx queue. Instead, call appropriate helper functions.
Signed-off-by: Masakazu Mokuno <mokuno@sm.sony.co.jp>
---
drivers/net/ps3_gelic_net.c | 124 +++++++++++++++++++++++++-------------------
1 file changed, 71 insertions(+), 53 deletions(-)
--- a/drivers/net/ps3_gelic_net.c
+++ b/drivers/net/ps3_gelic_net.c
@@ -408,22 +408,25 @@ static void gelic_net_release_tx_chain(s
break;
case GELIC_NET_DESCR_COMPLETE:
- card->netdev_stats.tx_packets++;
- card->netdev_stats.tx_bytes +=
- tx_chain->tail->skb->len;
+ if (tx_chain->tail->skb) {
+ card->netdev_stats.tx_packets++;
+ card->netdev_stats.tx_bytes +=
+ tx_chain->tail->skb->len;
+ }
break;
case GELIC_NET_DESCR_CARDOWNED:
/* pending tx request */
default:
/* any other value (== GELIC_NET_DESCR_NOT_IN_USE) */
- goto out;
+ if (!stop)
+ goto out;
}
gelic_net_release_tx_descr(card, tx_chain->tail);
- release = 1;
+ release ++;
}
out:
- if (!stop && release)
+ if (!stop && (2 < release))
netif_wake_queue(card->netdev);
}
@@ -660,19 +663,21 @@ static int gelic_net_prepare_tx_descr_v(
{
dma_addr_t buf[2];
unsigned int vlan_len;
+ struct gelic_net_descr *sec_descr = descr->next;
if (skb->len < GELIC_NET_VLAN_POS)
return -EINVAL;
- memcpy(&descr->vlan, skb->data, GELIC_NET_VLAN_POS);
+ vlan_len = GELIC_NET_VLAN_POS;
+ memcpy(&descr->vlan, skb->data, vlan_len);
if (card->vlan_index != -1) {
+ /* internal vlan tag used */
descr->vlan.h_vlan_proto = htons(ETH_P_8021Q); /* vlan 0x8100*/
descr->vlan.h_vlan_TCI = htons(card->vlan_id[card->vlan_index]);
- vlan_len = GELIC_NET_VLAN_POS + VLAN_HLEN; /* VLAN_HLEN=4 */
- } else
- vlan_len = GELIC_NET_VLAN_POS; /* no vlan tag */
+ vlan_len += VLAN_HLEN; /* added for above two lines */
+ }
- /* first descr */
+ /* map data area */
buf[0] = dma_map_single(ctodev(card), &descr->vlan,
vlan_len, DMA_TO_DEVICE);
@@ -683,20 +688,6 @@ static int gelic_net_prepare_tx_descr_v(
return -ENOMEM;
}
- descr->buf_addr = buf[0];
- descr->buf_size = vlan_len;
- descr->skb = skb; /* not used */
- descr->data_status = 0;
- gelic_net_set_txdescr_cmdstat(descr, skb, 1); /* not the frame end */
-
- /* second descr */
- card->tx_chain.head = card->tx_chain.head->next;
- descr->next_descr_addr = descr->next->bus_addr;
- descr = descr->next;
- if (gelic_net_get_descr_status(descr) != GELIC_NET_DESCR_NOT_IN_USE)
- /* XXX will be removed */
- dev_err(ctodev(card), "descr is not free!\n");
-
buf[1] = dma_map_single(ctodev(card), skb->data + GELIC_NET_VLAN_POS,
skb->len - GELIC_NET_VLAN_POS,
DMA_TO_DEVICE);
@@ -711,13 +702,24 @@ static int gelic_net_prepare_tx_descr_v(
return -ENOMEM;
}
- descr->buf_addr = buf[1];
- descr->buf_size = skb->len - GELIC_NET_VLAN_POS;
- descr->skb = skb;
+ /* first descr */
+ descr->buf_addr = buf[0];
+ descr->buf_size = vlan_len;
+ descr->skb = NULL; /* not used */
descr->data_status = 0;
- descr->next_descr_addr = 0; /* terminate hw descr */
- gelic_net_set_txdescr_cmdstat(descr, skb, 0);
+ descr->next_descr_addr = descr->next->bus_addr;
+ gelic_net_set_txdescr_cmdstat(descr, skb, 1); /* not the frame end */
+ /* second descr */
+ sec_descr->buf_addr = buf[1];
+ sec_descr->buf_size = skb->len - GELIC_NET_VLAN_POS;
+ sec_descr->skb = skb;
+ sec_descr->data_status = 0;
+ sec_descr->next_descr_addr = 0; /* terminate hw descr */
+ gelic_net_set_txdescr_cmdstat(sec_descr, skb, 0);
+
+ /* bump free descriptor pointer */
+ card->tx_chain.head = sec_descr->next;
return 0;
}
@@ -730,7 +732,7 @@ static int gelic_net_prepare_tx_descr_v(
static int gelic_net_kick_txdma(struct gelic_net_card *card,
struct gelic_net_descr *descr)
{
- int status = -ENXIO;
+ int status = 0;
int count = 10;
if (card->tx_dma_progress)
@@ -764,47 +766,62 @@ static int gelic_net_kick_txdma(struct g
static int gelic_net_xmit(struct sk_buff *skb, struct net_device *netdev)
{
struct gelic_net_card *card = netdev_priv(netdev);
- struct gelic_net_descr *descr = NULL;
+ struct gelic_net_descr *descr;
int result;
unsigned long flags;
spin_lock_irqsave(&card->tx_dma_lock, flags);
gelic_net_release_tx_chain(card, 0);
- if (!skb)
- goto kick;
+
descr = gelic_net_get_next_tx_descr(card);
if (!descr) {
+ /*
+ * no more descriptors free
+ */
netif_stop_queue(netdev);
spin_unlock_irqrestore(&card->tx_dma_lock, flags);
return NETDEV_TX_BUSY;
}
- result = gelic_net_prepare_tx_descr_v(card, descr, skb);
-
- if (result)
- goto error;
- card->tx_chain.head = card->tx_chain.head->next;
-
- if (descr->prev)
- descr->prev->next_descr_addr = descr->bus_addr;
-kick:
+ result = gelic_net_prepare_tx_descr_v(card, descr, skb);
+ if (result) {
+ /*
+ * DMA map failed. As chanses are that failure
+ * would continue, just release skb and return
+ */
+ card->netdev_stats.tx_dropped++;
+ dev_kfree_skb_any(skb);
+ spin_unlock_irqrestore(&card->tx_dma_lock, flags);
+ return NETDEV_TX_OK;
+ }
+ /*
+ * link this prepared descriptor to previous one
+ * to achieve high performance
+ */
+ descr->prev->next_descr_addr = descr->bus_addr;
/*
* as hardware descriptor is modified in the above lines,
* ensure that the hardware sees it
*/
wmb();
- if (gelic_net_kick_txdma(card, card->tx_chain.tail))
- goto error;
+ if (gelic_net_kick_txdma(card, descr)) {
+ /*
+ * kick failed.
+ * release descriptors which were just prepared
+ */
+ card->netdev_stats.tx_dropped++;
+ gelic_net_release_tx_descr(card, descr);
+ gelic_net_release_tx_descr(card, descr->next);
+ card->tx_chain.tail = descr->next->next;
+ dev_info(ctodev(card), "%s: kick failure\n", __func__);
+ } else {
+ /* OK, DMA started/reserved */
+ netdev->trans_start = jiffies;
+ }
- netdev->trans_start = jiffies;
spin_unlock_irqrestore(&card->tx_dma_lock, flags);
return NETDEV_TX_OK;
-
-error:
- card->netdev_stats.tx_dropped++;
- spin_unlock_irqrestore(&card->tx_dma_lock, flags);
- return NETDEV_TX_LOCKED;
}
/**
@@ -1025,9 +1042,10 @@ static irqreturn_t gelic_net_interrupt(i
if (status & GELIC_NET_TXINT) {
spin_lock_irqsave(&card->tx_dma_lock, flags);
card->tx_dma_progress = 0;
+ gelic_net_release_tx_chain(card, 0);
+ /* kick outstanding tx descriptor if any */
+ gelic_net_kick_txdma(card, card->tx_chain.tail);
spin_unlock_irqrestore(&card->tx_dma_lock, flags);
- /* start pending DMA */
- gelic_net_xmit(NULL, netdev);
}
return IRQ_HANDLED;
}
--
Masakazu MOKUNO
^ permalink raw reply
* [PATCH 4/10] ps3: removed defines no longer used
From: Masakazu Mokuno @ 2007-07-20 8:27 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, cbe-oss-dev
Removed defines no longer used.
Signed-off-by: Masakazu Mokuno <mokuno@sm.sony.co.jp>
---
drivers/net/ps3_gelic_net.h | 7 -------
1 file changed, 7 deletions(-)
--- a/drivers/net/ps3_gelic_net.h
+++ b/drivers/net/ps3_gelic_net.h
@@ -28,15 +28,8 @@
#ifndef _GELIC_NET_H
#define _GELIC_NET_H
-#define GELIC_NET_DRV_NAME "Gelic Network Driver"
-#define GELIC_NET_DRV_VERSION "1.0"
-
#define GELIC_NET_ETHTOOL /* use ethtool */
-/* ioctl */
-#define GELIC_NET_GET_MODE (SIOCDEVPRIVATE + 0)
-#define GELIC_NET_SET_MODE (SIOCDEVPRIVATE + 1)
-
/* descriptors */
#define GELIC_NET_RX_DESCRIPTORS 128 /* num of descriptors */
#define GELIC_NET_TX_DESCRIPTORS 128 /* num of descriptors */
--
Masakazu MOKUNO
^ permalink raw reply
* [PATCH 5/10] ps3: removed conditional ethtool support
From: Masakazu Mokuno @ 2007-07-20 8:28 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, cbe-oss-dev
Removed conditional ethtool support. Always enabled.
Signed-off-by: Masakazu Mokuno <mokuno@sm.sony.co.jp>
---
drivers/net/ps3_gelic_net.c | 4 ----
drivers/net/ps3_gelic_net.h | 2 --
2 files changed, 6 deletions(-)
--- a/drivers/net/ps3_gelic_net.c
+++ b/drivers/net/ps3_gelic_net.c
@@ -1160,7 +1160,6 @@ alloc_tx_failed:
return -ENOMEM;
}
-#ifdef GELIC_NET_ETHTOOL
static void gelic_net_get_drvinfo (struct net_device *netdev,
struct ethtool_drvinfo *info)
{
@@ -1280,7 +1279,6 @@ static struct ethtool_ops gelic_net_etht
.get_rx_csum = gelic_net_get_rx_csum,
.set_rx_csum = gelic_net_set_rx_csum,
};
-#endif
/**
* gelic_net_tx_timeout_task - task scheduled by the watchdog timeout
@@ -1348,9 +1346,7 @@ static void gelic_net_setup_netdev_ops(s
/* NAPI */
netdev->poll = &gelic_net_poll;
netdev->weight = GELIC_NET_NAPI_WEIGHT;
-#ifdef GELIC_NET_ETHTOOL
netdev->ethtool_ops = &gelic_net_ethtool_ops;
-#endif
}
/**
--- a/drivers/net/ps3_gelic_net.h
+++ b/drivers/net/ps3_gelic_net.h
@@ -28,8 +28,6 @@
#ifndef _GELIC_NET_H
#define _GELIC_NET_H
-#define GELIC_NET_ETHTOOL /* use ethtool */
-
/* descriptors */
#define GELIC_NET_RX_DESCRIPTORS 128 /* num of descriptors */
#define GELIC_NET_TX_DESCRIPTORS 128 /* num of descriptors */
--
Masakazu MOKUNO
^ permalink raw reply
* [PATCH 6/10] ps3: use net_device_stats of net_device structure
From: Masakazu Mokuno @ 2007-07-20 8:31 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, cbe-oss-dev
Removed the statistics information from private structre.
Instead, use net_device_stats in net_device structure.
Signed-off-by: Masakazu Mokuno <mokuno@sm.sony.co.jp>
---
drivers/net/ps3_gelic_net.c | 31 ++++++++-----------------------
drivers/net/ps3_gelic_net.h | 1 -
2 files changed, 8 insertions(+), 24 deletions(-)
--- a/drivers/net/ps3_gelic_net.c
+++ b/drivers/net/ps3_gelic_net.c
@@ -404,13 +404,13 @@ static void gelic_net_release_tx_chain(s
"%s: forcing end of tx descriptor " \
"with status %x\n",
__func__, status);
- card->netdev_stats.tx_dropped++;
+ card->netdev->stats.tx_dropped++;
break;
case GELIC_NET_DESCR_COMPLETE:
if (tx_chain->tail->skb) {
- card->netdev_stats.tx_packets++;
- card->netdev_stats.tx_bytes +=
+ card->netdev->stats.tx_packets++;
+ card->netdev->stats.tx_bytes +=
tx_chain->tail->skb->len;
}
break;
@@ -790,7 +790,7 @@ static int gelic_net_xmit(struct sk_buff
* DMA map failed. As chanses are that failure
* would continue, just release skb and return
*/
- card->netdev_stats.tx_dropped++;
+ card->netdev->stats.tx_dropped++;
dev_kfree_skb_any(skb);
spin_unlock_irqrestore(&card->tx_dma_lock, flags);
return NETDEV_TX_OK;
@@ -810,7 +810,7 @@ static int gelic_net_xmit(struct sk_buff
* kick failed.
* release descriptors which were just prepared
*/
- card->netdev_stats.tx_dropped++;
+ card->netdev->stats.tx_dropped++;
gelic_net_release_tx_descr(card, descr);
gelic_net_release_tx_descr(card, descr->next);
card->tx_chain.tail = descr->next->next;
@@ -872,8 +872,8 @@ static void gelic_net_pass_skb_up(struct
skb->ip_summed = CHECKSUM_NONE;
/* update netdevice statistics */
- card->netdev_stats.rx_packets++;
- card->netdev_stats.rx_bytes += skb->len;
+ card->netdev->stats.rx_packets++;
+ card->netdev->stats.rx_bytes += skb->len;
/* pass skb up to stack */
netif_receive_skb(skb);
@@ -913,7 +913,7 @@ static int gelic_net_decode_one_descr(st
(status == GELIC_NET_DESCR_FORCE_END)) {
dev_info(ctodev(card), "dropping RX descriptor with state %x\n",
status);
- card->netdev_stats.rx_dropped++;
+ card->netdev->stats.rx_dropped++;
goto refill;
}
@@ -986,20 +986,6 @@ static int gelic_net_poll(struct net_dev
} else
return 1;
}
-
-/**
- * gelic_net_get_stats - get interface statistics
- * @netdev: interface device structure
- *
- * returns the interface statistics residing in the gelic_net_card struct
- */
-static struct net_device_stats *gelic_net_get_stats(struct net_device *netdev)
-{
- struct gelic_net_card *card = netdev_priv(netdev);
-
- return &card->netdev_stats;
-}
-
/**
* gelic_net_change_mtu - changes the MTU of an interface
* @netdev: interface device structure
@@ -1337,7 +1323,6 @@ static void gelic_net_setup_netdev_ops(s
netdev->open = &gelic_net_open;
netdev->stop = &gelic_net_stop;
netdev->hard_start_xmit = &gelic_net_xmit;
- netdev->get_stats = &gelic_net_get_stats;
netdev->set_multicast_list = &gelic_net_set_multi;
netdev->change_mtu = &gelic_net_change_mtu;
/* tx watchdog */
--- a/drivers/net/ps3_gelic_net.h
+++ b/drivers/net/ps3_gelic_net.h
@@ -210,7 +210,6 @@ struct gelic_net_card {
/* gurad dmac descriptor chain*/
spinlock_t chain_lock;
- struct net_device_stats netdev_stats;
int rx_csum;
/* guard tx_dma_progress */
spinlock_t tx_dma_lock;
--
Masakazu MOKUNO
^ permalink raw reply
* [PATCH 7/10] ps3: use ethX as the name of irq
From: Masakazu Mokuno @ 2007-07-20 8:33 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, cbe-oss-dev
Use net_device name for registration of irq as many network drivers do.
Signed-off-by: Masakazu Mokuno <mokuno@sm.sony.co.jp>
---
drivers/net/ps3_gelic_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/net/ps3_gelic_net.c
+++ b/drivers/net/ps3_gelic_net.c
@@ -1073,7 +1073,7 @@ static int gelic_net_open_device(struct
}
result = request_irq(card->netdev->irq, gelic_net_interrupt,
- IRQF_DISABLED, "gelic network", card->netdev);
+ IRQF_DISABLED, card->netdev->name, card->netdev);
if (result) {
dev_info(ctodev(card), "%s:%d: request_irq failed (%d)\n",
--
Masakazu MOKUNO
^ permalink raw reply
* [PATCH 8/10] ps3: removed calling netif_poll_enable() in open()
From: Masakazu Mokuno @ 2007-07-20 8:34 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, cbe-oss-dev
Removed use of netif_poll_enable() in open function.
Signed-off-by: Masakazu Mokuno <mokuno@sm.sony.co.jp>
---
drivers/net/ps3_gelic_net.c | 1 -
1 file changed, 1 deletion(-)
--- a/drivers/net/ps3_gelic_net.c
+++ b/drivers/net/ps3_gelic_net.c
@@ -1134,7 +1134,6 @@ static int gelic_net_open(struct net_dev
netif_start_queue(netdev);
netif_carrier_on(netdev);
- netif_poll_enable(netdev);
return 0;
--
Masakazu MOKUNO
^ permalink raw reply
* Re: Problems receving multicasts with r8169.c driver from linux kernel (2.6.21.5)
From: Stephen Hemminger @ 2007-07-20 8:35 UTC (permalink / raw)
To: Reither Robert; +Cc: Francois Romieu, netdev
In-Reply-To: <BA41E8A07B73A041948C2E3A4EF6239D062E05@av-digital-exch.avdigital.at>
On Fri, 20 Jul 2007 09:19:15 +0200
"Reither Robert" <Robert.Reither@av-digital.at> wrote:
>
> Hi Francois ,
>
> i can spare some time now, so i ask:
> Would it help, if i code/send u a tool to simulate your multicast packet scenario ?
> Buts its possible for me to reproduce the problem with e.g. nc -u 224.1.1.1 10500 <dummy_file (does show me the same bad behaviour).
> Or can i help in another way ?
iperf also works as simple multicast test.
-----------------------------------------------
http://dast.nlanr.net/Projects/Iperf/
Running the multicast server and client
Use the -B option while starting the server to bind it to a multicast address.
E.g. :-iperf -s -u -B 224.0.55.55 -i 1.
This will have the Iperf server listening for datagrams (-u) from the address 224.0.55.55(-B 224.0.55.55), with a periodic reporting interval of 1s (-i 1).
Now, start a client sending packets to this multicast address, with a TTL depending on your Network Topology (if you are unsure, use a high TTL like 64 or higher).
E.g. : iperf -c 224.0.55.55 -u -T 32 -t 10 -i 1. This will have a UDP client (-u) connected to the multicast address 224.0.55.55(-c 224.0.55.55), with a TTL of 32 (-T 32), sending data for 10 seconds (-t 10), with a periodic reporting interval of 1s (-i 1).
Start multiple clients as explained above, sending data to the same multicast server. (If you have multiple servers listening on the multicast address, each of the servers will be getting the data)
^ permalink raw reply
* [PATCH 9/10] ps3: fix rare issue that reenabling rx DMA fails
From: Masakazu Mokuno @ 2007-07-20 8:35 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, cbe-oss-dev
Fixed rare issue that 'lv1_net_start_rx_dma failed, status=-9" was shown
in dmesg. This meant restarting rx DMA had been rejected by the hypervisor.
This issue would caused if the guest os requested starting DMA when
the hypervisor thought the DMA was in progress.
The state machine for DMA status of the hypervisor would be updated
by processing interrupt in the hypervisor.
Thus we should wait for the interrupt delivery before restarting
DMA.
Signed-off-by: Masakazu Mokuno <mokuno@sm.sony.co.jp>
---
drivers/net/ps3_gelic_net.c | 9 +++++++--
drivers/net/ps3_gelic_net.h | 1 +
2 files changed, 8 insertions(+), 2 deletions(-)
--- a/drivers/net/ps3_gelic_net.c
+++ b/drivers/net/ps3_gelic_net.c
@@ -943,8 +943,8 @@ refill:
descr->prev->next_descr_addr = descr->bus_addr;
if (dmac_chain_ended) {
- gelic_net_enable_rxdmac(card);
- dev_dbg(ctodev(card), "reenable rx dma\n");
+ card->rx_dma_restart_required = 1;
+ dev_dbg(ctodev(card), "reenable rx dma scheduled\n");
}
return 1;
@@ -1020,6 +1020,11 @@ static irqreturn_t gelic_net_interrupt(i
if (!status)
return IRQ_NONE;
+ if (card->rx_dma_restart_required) {
+ card->rx_dma_restart_required = 0;
+ gelic_net_enable_rxdmac(card);
+ }
+
if (status & GELIC_NET_RXINT) {
gelic_net_rx_irq_off(card);
netif_rx_schedule(netdev);
--- a/drivers/net/ps3_gelic_net.h
+++ b/drivers/net/ps3_gelic_net.h
@@ -207,6 +207,7 @@ struct gelic_net_card {
struct gelic_net_descr_chain tx_chain;
struct gelic_net_descr_chain rx_chain;
+ int rx_dma_restart_required;
/* gurad dmac descriptor chain*/
spinlock_t chain_lock;
--
Masakazu MOKUNO
^ permalink raw reply
* [PATCH 10/10] ps3: reduce allocation size of rx skb buffers
From: Masakazu Mokuno @ 2007-07-20 8:39 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, cbe-oss-dev
Reduced allocation size for rx skb buffers, from 2308 bytes to
1356 per buffer.
Signed-off-by: Masakazu Mokuno <mokuno@sm.sony.co.jp>
---
drivers/net/ps3_gelic_net.c | 45 ++++++++++++++++++++++++++++++++++++--------
drivers/net/ps3_gelic_net.h | 12 ++++++-----
2 files changed, 44 insertions(+), 13 deletions(-)
--- a/drivers/net/ps3_gelic_net.c
+++ b/drivers/net/ps3_gelic_net.c
@@ -917,31 +917,60 @@ static int gelic_net_decode_one_descr(st
goto refill;
}
- if ((status != GELIC_NET_DESCR_COMPLETE) &&
- (status != GELIC_NET_DESCR_FRAME_END)) {
+ if (status == GELIC_NET_DESCR_BUFFER_FULL) {
+ /*
+ * Buffer full would occur if and only if
+ * the frame length was longer than the size of this
+ * descriptor's buffer. If the frame length was equal
+ * to or shorter than buffer'size, FRAME_END condition
+ * would occur.
+ * Anyway this frame was longer than the MTU,
+ * just drop it.
+ */
+ dev_info(ctodev(card), "overlength frame\n");
+ goto refill;
+ }
+ /*
+ * descriptoers any other than FRAME_END here should
+ * be treated as error.
+ */
+ if (status != GELIC_NET_DESCR_FRAME_END) {
dev_dbg(ctodev(card), "RX descriptor with state %x\n",
status);
goto refill;
}
/* ok, we've got a packet in descr */
- gelic_net_pass_skb_up(descr, card); /* 1: skb_up sccess */
-
+ gelic_net_pass_skb_up(descr, card);
refill:
- descr->next_descr_addr = 0; /* unlink the descr */
+ /*
+ * So that always DMAC can see the end
+ * of the descriptor chain to avoid
+ * from unwanted DMAC overrun.
+ */
+ descr->next_descr_addr = 0;
/* change the descriptor state: */
gelic_net_set_descr_status(descr, GELIC_NET_DESCR_NOT_IN_USE);
- /* refill one desc
- * FIXME: this can fail, but for now, just leave this
- * descriptor without skb
+ /*
+ * this call can fail, but for now, just leave this
+ * decriptor without skb
*/
gelic_net_prepare_rx_descr(card, descr);
+
chain->head = descr;
chain->tail = descr->next;
+
+ /*
+ * Set this descriptor the end of the chain.
+ */
descr->prev->next_descr_addr = descr->bus_addr;
+ /*
+ * If dmac chain was met, DMAC stopped.
+ * thus re-enable it
+ */
if (dmac_chain_ended) {
card->rx_dma_restart_required = 1;
dev_dbg(ctodev(card), "reenable rx dma scheduled\n");
--- a/drivers/net/ps3_gelic_net.h
+++ b/drivers/net/ps3_gelic_net.h
@@ -32,8 +32,8 @@
#define GELIC_NET_RX_DESCRIPTORS 128 /* num of descriptors */
#define GELIC_NET_TX_DESCRIPTORS 128 /* num of descriptors */
-#define GELIC_NET_MAX_MTU 2308
-#define GELIC_NET_MIN_MTU 64
+#define GELIC_NET_MAX_MTU VLAN_ETH_FRAME_LEN
+#define GELIC_NET_MIN_MTU VLAN_ETH_ZLEN
#define GELIC_NET_RXBUF_ALIGN 128
#define GELIC_NET_RX_CSUM_DEFAULT 1 /* hw chksum */
#define GELIC_NET_WATCHDOG_TIMEOUT 5*HZ
@@ -81,7 +81,8 @@ enum gelic_net_int1_status {
*/
#define GELIC_NET_RXVLNPKT 0x00200000 /* VLAN packet */
/* bit 20..16 reserved */
-#define GELIC_NET_RXRECNUM 0x0000ff00 /* reception receipt number */
+#define GELIC_NET_RXRRECNUM 0x0000ff00 /* reception receipt number */
+#define GELIC_NET_RXRRECNUM_SHIFT 8
/* bit 7..0 reserved */
#define GELIC_NET_TXDESC_TAIL 0
@@ -129,13 +130,14 @@ enum gelic_net_int1_status {
enum gelic_net_descr_status {
- GELIC_NET_DESCR_COMPLETE = 0x00, /* used in rx and tx */
+ GELIC_NET_DESCR_COMPLETE = 0x00, /* used in tx */
+ GELIC_NET_DESCR_BUFFER_FULL = 0x00, /* used in rx */
GELIC_NET_DESCR_RESPONSE_ERROR = 0x01, /* used in rx and tx */
GELIC_NET_DESCR_PROTECTION_ERROR = 0x02, /* used in rx and tx */
GELIC_NET_DESCR_FRAME_END = 0x04, /* used in rx */
GELIC_NET_DESCR_FORCE_END = 0x05, /* used in rx and tx */
GELIC_NET_DESCR_CARDOWNED = 0x0a, /* used in rx and tx */
- GELIC_NET_DESCR_NOT_IN_USE /* any other value */
+ GELIC_NET_DESCR_NOT_IN_USE = 0x0b /* any other value */
};
/* for lv1_net_control */
#define GELIC_NET_GET_MAC_ADDRESS 0x0000000000000001
--
Masakazu MOKUNO
^ permalink raw reply
* [Patch] Make ULA flagged unicast global
From: Arnaud Ebalard @ 2007-07-20 8:54 UTC (permalink / raw)
To: netdev
Hi,
Find attached a patch to get IPv6 Unique Local Addresses (FC00::/7)
flagged unicast global in __ipv6_addr_type(), as expected by RFC 4193.
One easy way to see the current difference of handling with a more
common unicast global address is by trying to insert a default route
using a unique local address for the gateway:
$ sudo ip -6 addr add fd00::1/64 dev eth0
$ sudo ip -6 route add default via fd00::2 dev eth0
RTNETLINK answers: Invalid argument
where sth like 2001:db8::1 (in fact, where first 3 bits are different of
000 or 111) does work.
The patch is against Linus git tree but the code in the modified file is
pretty stable so it should apply without problems.
One remark: as ULA get flagged as unicast global by that change, there
might be a difference in address selection mechanism. Anyway, the
longest prefix match rule should do his job if something better is
available, i.e. a unique local address will not be selected as src or
dst for something in 2000::/3, for instance.
Regards,
a+
ps : i'm aware LL addresses should be used for expressing gw, not global
ones.
diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c
index faaefb6..93b17d5 100644
--- a/net/ipv6/addrconf_core.c
+++ b/net/ipv6/addrconf_core.c
@@ -29,11 +29,13 @@ int __ipv6_addr_type(const struct in6_addr *addr)
st = addr->s6_addr32[0];
- /* Consider all addresses with the first three bits different of
- 000 and 111 as unicasts.
+ /* - Consider all addresses with the first three bits different of
+ 000 and 111 as unicasts.
+ - Unique Local Addresses (FC00::/7, RFC 4193) are unicast global.
*/
- if ((st & htonl(0xE0000000)) != htonl(0x00000000) &&
- (st & htonl(0xE0000000)) != htonl(0xE0000000))
+ if (((st & htonl(0xE0000000)) != htonl(0x00000000) &&
+ (st & htonl(0xE0000000)) != htonl(0xE0000000)) ||
+ ((st & htonl(0xFE000000)) == htonl(0xFC000000)))
return (IPV6_ADDR_UNICAST |
IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_GLOBAL));
^ permalink raw reply related
* [PATCH] NET: Add missing entries to family name tables
From: David Howells @ 2007-07-20 9:53 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, dhowells
Add missing entries to af_family_clock_key_strings[].
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/core/sock.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/core/sock.c b/net/core/sock.c
index bd209c4..cfed7d4 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -182,7 +182,8 @@ static const char *af_family_clock_key_strings[AF_MAX+1] = {
"clock-21" , "clock-AF_SNA" , "clock-AF_IRDA" ,
"clock-AF_PPPOX" , "clock-AF_WANPIPE" , "clock-AF_LLC" ,
"clock-27" , "clock-28" , "clock-29" ,
- "clock-AF_TIPC" , "clock-AF_BLUETOOTH", "clock-AF_MAX"
+ "clock-AF_TIPC" , "clock-AF_BLUETOOTH", "clock-AF_IUCV" ,
+ "clock-AF_RXRPC" , "clock-AF_MAX"
};
#endif
^ permalink raw reply related
* Re: [PATCH 02/10] Networking include file changes.
From: Patrick McHardy @ 2007-07-20 9:59 UTC (permalink / raw)
To: Krishna Kumar
Cc: davem, rdreier, johnpol, Robert.Olsson, peter.p.waskiewicz.jr,
herbert, gaagaan, kumarkr, xma, rick.jones2, mcarlson, netdev,
jagana, general, mchan, tgraf, jeff, hadi, sri
In-Reply-To: <20070720063216.26341.80316.sendpatchset@localhost.localdomain>
Krishna Kumar wrote:
> diff -ruNp org/include/linux/netdevice.h new/include/linux/netdevice.h
> --- org/include/linux/netdevice.h 2007-07-20 07:49:28.000000000 +0530
> +++ new/include/linux/netdevice.h 2007-07-20 08:30:55.000000000 +0530
> @@ -264,6 +264,8 @@ enum netdev_state_t
> __LINK_STATE_QDISC_RUNNING,
> };
>
> +/* Minimum length of device hardware queue for batching to work */
> +#define MIN_QUEUE_LEN_BATCH 16
Is there any downside in using batching with smaller queue sizes?
^ permalink raw reply
* [ofa-general] Re: [PATCH 03/10] dev.c changes.
From: Patrick McHardy @ 2007-07-20 10:04 UTC (permalink / raw)
To: Krishna Kumar
Cc: johnpol, Robert.Olsson, peter.p.waskiewicz.jr, herbert, gaagaan,
kumarkr, rdreier, mcarlson, jagana, general, netdev, tgraf, jeff,
hadi, davem, mchan, sri
In-Reply-To: <20070720063227.26341.91868.sendpatchset@localhost.localdomain>
Krishna Kumar wrote:
> @@ -3397,6 +3440,28 @@ int register_netdevice(struct net_device
> }
> }
>
> + if (dev->features & NETIF_F_BATCH_SKBS) {
> + if (!dev->hard_start_xmit_batch ||
> + dev->tx_queue_len < MIN_QUEUE_LEN_BATCH) {
> + /*
> + * Batch TX requires API support in driver plus have
> + * a minimum sized queue.
> + */
> + printk(KERN_ERR "%s: Dropping NETIF_F_BATCH_SKBS "
> + "since no API support or queue len "
> + "is smaller than %d.\n",
> + dev->name, MIN_QUEUE_LEN_BATCH);
> + dev->features &= ~NETIF_F_BATCH_SKBS;
The queue length can be changed through multiple interfaces, if that
really is important you need to catch these cases too.
> + } else {
> + dev->skb_blist = kmalloc(sizeof *dev->skb_blist,
> + GFP_KERNEL);
Why not simply put the head in struct net_device? It seems to me that
this could also be used for gso_skb.
> + if (dev->skb_blist) {
> + skb_queue_head_init(dev->skb_blist);
> + dev->tx_queue_len >>= 1;
> + }
> + }
> + }
> +
> /*
> * nil rebuild_header routine,
> * that should be never called and used as just bug trap.
> @@ -3732,10 +3797,16 @@ void unregister_netdevice(struct net_dev
>
> synchronize_net();
>
> + /* Deallocate batching structure */
> + if (dev->skb_blist) {
> + skb_queue_purge(dev->skb_blist);
> + kfree(dev->skb_blist);
> + dev->skb_blist = NULL;
> + }
> +
Queue purging should be done in dev_deactivate.
^ permalink raw reply
* [ofa-general] Re: [PATCH 04/10] net-sysfs.c changes.
From: Patrick McHardy @ 2007-07-20 10:07 UTC (permalink / raw)
To: Krishna Kumar
Cc: johnpol, Robert.Olsson, jeff, herbert, gaagaan, kumarkr, rdreier,
peter.p.waskiewicz.jr, mcarlson, jagana, general, mchan, tgraf,
netdev, sri, hadi, davem
In-Reply-To: <20070720063238.26341.41474.sendpatchset@localhost.localdomain>
Krishna Kumar wrote:
> Support to turn on/off batching from /sys.
rtnetlink support seems more important than sysfs to me.
^ permalink raw reply
* [ofa-general] Re: [PATCH 05/10] sch_generic.c changes.
From: Patrick McHardy @ 2007-07-20 10:11 UTC (permalink / raw)
To: Krishna Kumar
Cc: johnpol, Robert.Olsson, peter.p.waskiewicz.jr, herbert, gaagaan,
kumarkr, rdreier, mcarlson, netdev, jagana, general, mchan, tgraf,
jeff, hadi, davem, sri
In-Reply-To: <20070720063249.26341.125.sendpatchset@localhost.localdomain>
Krishna Kumar wrote:
> diff -ruNp org/net/sched/sch_generic.c new/net/sched/sch_generic.c
> --- org/net/sched/sch_generic.c 2007-07-20 07:49:28.000000000 +0530
> +++ new/net/sched/sch_generic.c 2007-07-20 08:30:22.000000000 +0530
> @@ -9,6 +9,11 @@
> * Authors: Alexey Kuznetsov, <kuznet@ms2.inr.ac.ru>
> * Jamal Hadi Salim, <hadi@cyberus.ca> 990601
> * - Ingress support
> + *
> + * New functionality:
> + * Krishna Kumar, <krkumar2@in.ibm.com>, July 2007
> + * - Support for sending multiple skbs to devices that support
> + * new api - dev->hard_start_xmit_batch()
No new changelogs in source code please, git keeps track of that.
> -static inline int qdisc_restart(struct net_device *dev)
> +static inline int qdisc_restart(struct net_device *dev,
> + struct sk_buff_head *blist)
> {
> struct Qdisc *q = dev->qdisc;
> struct sk_buff *skb;
> - unsigned lockless;
> + unsigned getlock; /* whether we need to get lock or not */
Unrelated rename, please get rid of this to reduce the noise.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox