Netdev List
 help / color / mirror / Atom feed
* RE: [5/8,RFC] CAIF Protocol Stack
From: Sjur Brændeland @ 2009-10-06 19:01 UTC (permalink / raw)
  To: Stefano Babic; +Cc: netdev, Kim Lilliestierna XX
In-Reply-To: <4ACA0DB7.5000802@babic.homelinux.org>

Stefano Babic wrote:
>> +obj-$(CAIF_CHRDEV) += chnl_chr.o
> Probably this should be obj-$(CONFIG_CAIF_CHARDEV) += chnl_chr.o
>> +obj-$(CAIF_NETDEV) += chnl_net.o
>Should be maybe CONFIG_CAIF_NETDEV ?

Thanks, it will be fixed in the patch-set sent out this week.

BR/Sjur Brændeland

^ permalink raw reply

* RE: [1/8,RFC] CAIF Protocol Stack
From: Sjur Brændeland @ 2009-10-06 19:00 UTC (permalink / raw)
  To: Stefano Babic; +Cc: netdev, Kim Lilliestierna XX
In-Reply-To: <4AC9ECD4.70602@babic.homelinux.org>

Hi Stefano.
Thank you for useful feedback!
I'm planning to send out a new patch-set sometime this week.

Stefano Babic wrote:
> [snip]
> +union caif_action {
>... 
> It seems this structure is defined twice (here and in caif_actions.h)

I have cleaned this up, caif_action is now only defined in caif_actions.h


BR/Sjur Brændeland

^ permalink raw reply

* Re: Support of multiple default routes in Linux ?
From: Ben Greear @ 2009-10-06 18:02 UTC (permalink / raw)
  To: Mallika Gautam; +Cc: linux-net, netdev
In-Reply-To: <9041a1390910060837g1a536bc3x624cc6a5026af83d@mail.gmail.com>

On 10/06/2009 08:37 AM, Mallika Gautam wrote:
> Hi All,
>
> I am working on 2.6.26 kernel. I am trying to setup Policy based
> routing with multiple IP addresses in same subnet, with multiple
> routing tables, each having its own default gateway. I have no route
> in the 'Main' table. Problem is that I am not able to add default
> route in each of the routing table.
>
> I have 3 interfaces in the same subnet, sharing the gateway. I am
> using 3 different routing tables for them. I need to add this gateway
> to each of the routing tables with different interface associated with
> the routing table.
>
> Routing tables look like this -
>
> eth0: 192.168.1.1/16
> eth1: 192.168.1.2/16
> eth2: 192.168.1.3/16
>
> main table: empty
>
> ---- eth0tbl: routing table for eth0 ----
> 192.168.0.0/16 dev eth0
>
> --- eth1tbl: routing table for eth1 ---
> 192.168.0.0/16 dev eth1
>
> --- eth2tbl: routing table for eth2 ---
> 192.168.0.0/16 dev eth2
>
> #ip rule
> 0: from all lookup local
> 2: from 192.168.1.1 iif lo lookup eth0tbl
> 3: from all to 192.168.1.1 iif eth0 lookup eth0tbl
> 4: from 192.168.1.2 iif lo lookup eth1tbl
> 5: from all to 192.168.1.2 iif eth1 lookup eth1tbl
> 6: from 192.168.1.3 iif lo lookup eth2tbl
> 7: from all to 192.168.1.3 iif eth2 lookup eth2tbl
> 32766: from all lookup main ---------->  /* empty */
> 32767: from all lookup default ---------->  /* empty */
>
> when I give following command to add default route, it returns error -
> #ip route add default via 192.168.254.254 dev eth0 table eth0tbl
> RTNETLINK answers: No such process.
>
> #ip route add default via 192.168.254.254 dev eth1 table eth1tbl
> RTNETLINK answers: No such process.
>
> #ip route add default via 192.168.254.254 dev eth2 table eth2tbl
> RTNETLINK answers: No such process.
>
> arp_announce, arp_ignore and arp_filter are all set to 1.
> Is there a way this can be achieved? Is this available in any of the
> later kernel versions? Any pointers would be of help.

Try adding subnet routes first?  This can work, as we do it all
the time.

We also use numeric tags for table-id, but no idea if that is an issue.

Ben

>
> Thanks
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply

* Re: [PATCH RFC] isdn/capi: fix up CAPI subsystem workaround locking a bit
From: Tilman Schmidt @ 2009-10-06 17:52 UTC (permalink / raw)
  To: Michael Buesch
  Cc: i4ldeveloper, Carsten Paeth, Karsten Keil, Karsten Keil,
	Armin Schindler, isdn4linux, netdev, linux-kernel
In-Reply-To: <200910052324.09992.mb@bu3sch.de>

[-- Attachment #1: Type: text/plain, Size: 2895 bytes --]

On Mon, 2009-10-05 23:24:07 +0200, Michael Buesch wrote:
> On Monday 05 October 2009 13:42:29 Tilman Schmidt wrote:

>> Thanks for the info. So do I understand correctly that after:
>>
>> commit 6aa65472d18703064898eefb5eb58f7ecd0d8912
>> Author: Michael Buesch <mb@bu3sch.de>
>> Date:   Mon Jun 26 00:25:30 2006 -0700
>>
>>     [PATCH] CAPI crash / race condition
>>
>> you were actually still seeing LIST_POISON2 Oopses in
>> capiminor_del_ack(), but after:
> 
> Yeah well. The oops with LIST_POISON was with a patch that converted the
> datahandle_queue to struct list_head, but without the spinlock_t ackqlock added.
> Then I added the spinlock_t ackqlock and it first seemed to fix the problem. (That
> is the patch from the mail).

Understood.

> But it did only shrink the race window, so the crash did still happen, but less often.

Ok.

> The crash was only "fixed" with the workaround_lock patch (but _without_ any of the
> ackqueue patches applied.)
> 
>> commit 053b47ff249b9e0a634dae807f81465205e7c228
>> Author: Michael Buesch <mb@bu3sch.de>
>> Date:   Mon Feb 12 00:53:26 2007 -0800
>>
>>     [PATCH] Workaround CAPI subsystem locking issue
>>
>> they were gone? That's interesting. I'll try to wrap my mind around
>> this.
> 
> Yeah, this sledgehammer lock did fix the crash while leaving the old non-list-head
> queue in place (it should still be there today).

That is not the case. In mainline, your second patch was applied on
top of the first one, so there is now struct list_head ackqueue and
spinlock_t ackqlock as well as static DEFINE_SPINLOCK(workaround_lock).

>> It's unfortunate that these crashes only seem to occur with one specific
>> device (FritzCard DSL) which I don't have.
> 
> I still have the device somewhere. If you want to have it, I can blow off the
> dust and send it to you. If you don't want it, I'll throw it away soon.
> I'd really like to send it to you to get rid of it. ;)

Feel free to do so. I'll send you my snailmail address by PM. I'm
making no promises about when I might get around to actually testing
it, though.

>> Can anyone shed some light on 
>> what that device is doing differently from other ISDN cards?
> 
> Well, it's a combined ISDN/DSL card, but I never used the ISDN part. So the crash
> happened while transferring data over the DSL link.

DSL over CAPI? Strange.

> The vendor driver is closed source with an open wrapper (like nvidia). It's a pretty
> crappy unmaintained piece of software, but it ran stable with some patches applied
> to the driver and the workaround-lock patch to the capi stack.

O dear. I wonder what I'll find.

Thanks,
Tilman

-- 
Tilman Schmidt                    E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

^ permalink raw reply

* Re: [PATCH] iproute2 add hoplimit parsing and update usage and documentation
From: Stephen Hemminger @ 2009-10-06 17:34 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Gilad Ben-Yossef, netdev, Ori Finkelman, Andreas Henriksson
In-Reply-To: <4ACB7AF4.4070207@gmail.com>

On Tue, 06 Oct 2009 19:14:28 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Gilad Ben-Yossef a écrit :
> > From: Gilad Ben-Yossef <gilad@codefidence.com>
> > 
> > - Parse and handle the hoplimit ip route option and add it to the usage 
> >   line and documentation.
> > 
> > - Add the missing reordering ip route option to the usage line.
> > 
> > - Add documentation for initcwnd ip route option.
> > 
> > Tested by setting hoplimit and retreiving it via "show". 
> > 
> > Signed-off-by: Gilad Ben-Yossef <gilad@codefidence.com>
> > [ported to HEAD, fixed a bug with hoplimit lock handling, added documentation]
> > Signed-off-by: Ori Finkelman <ori@comsleep.com>
> > Signed-off-by: Yony Amit <yony@comsleep.com>
> > 
> 
> > --- a/ip/iproute.c
> > +++ b/ip/iproute.c
> > @@ -13,6 +13,8 @@
> >   *
> >   * Rani Assaf <rani@magic.metawire.com> 980929:	resolve addresses
> >   * Kunihiro Ishiguro <kunihiro@zebra.org> 001102: rtnh_ifindex was not initialized
> > + * Gilad Ben-Yossef <gilad@codefidence.com> 091006: Add hoplimit option parsing 
> > + * and correct usage
> 
> Well, git/web history is much better than this, you dont need to add this comment.
> 
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> patch should be sent to iproute2 maintainer, Stephen Hemminger <shemminger@vyatta.com>
> 
> He might be too busy to catch netdev mails these days, I dont know :)
> 
> Thanks

I catch them, but collect iproute patches and put them in batches around
release time.

-- 

^ permalink raw reply

* [PATCH net-next] myri10ge: add adaptive coalescing
From: Brice Goglin @ 2009-10-06 16:52 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Network Development list, Andrew J. Gallatin

This patch adds support for adaptive interrupt coalescing to the
myri10ge driver. It is based on the host periodically look at
statistics and update the NIC coalescing accordingly.

The NIC only provides packet throughput and we feel that it is a
better heuristics than the packet rate heuristics currently used
in ethtool. Also, assuming that the packet packet rate heuristics
uses what is actually sent on the wire when using TSO, it would be
much more expensive to implement correctly, as the driver would
need to calculate how many packets were sent.

Signed-off-by: Andrew Gallatin <gallatin@myri.com>
Signed-off-by: Brice Goglin <brice@myri.com>

--- linux-tmp.20091006.1050.49/drivers/net/myri10ge/myri10ge.c	2009-10-06 10:50:28.000000000 +0200
+++ linux-tmp/drivers/net/myri10ge/myri10ge.c	2009-10-06 17:17:08.000000000 +0200
@@ -75,7 +75,7 @@
 #include "myri10ge_mcp.h"
 #include "myri10ge_mcp_gen_header.h"
 
-#define MYRI10GE_VERSION_STR "1.5.0-1.432"
+#define MYRI10GE_VERSION_STR "1.5.1-1.446"
 
 MODULE_DESCRIPTION("Myricom 10G driver (10GbE)");
 MODULE_AUTHOR("Maintainer: help@myri.com");
@@ -83,6 +83,7 @@
 MODULE_LICENSE("Dual BSD/GPL");
 
 #define MYRI10GE_MAX_ETHER_MTU 9014
+#define MYRI10GE_INTR_COAL_PERIOD 4
 
 #define MYRI10GE_ETH_STOPPED 0
 #define MYRI10GE_ETH_STOPPING 1
@@ -197,6 +198,15 @@
 	char irq_desc[32];
 };
 
+struct myri10ge_adapt_intr_coal {
+	int enabled;
+	int usecs;
+	int big_usecs;
+	unsigned long old_tx_bytes;
+	unsigned long old_rx_bytes;
+	struct timer_list timer;
+};
+
 struct myri10ge_priv {
 	struct myri10ge_slice_state *ss;
 	int tx_boundary;	/* boundary transmits cannot cross */
@@ -228,6 +238,7 @@
 	unsigned int rdma_tags_available;
 	int intr_coal_delay;
 	__be32 __iomem *intr_coal_delay_ptr;
+	struct myri10ge_adapt_intr_coal adapt_coal;
 	int mtrr;
 	int wc_enabled;
 	int down_cnt;
@@ -292,6 +303,16 @@
 module_param(myri10ge_intr_coal_delay, int, S_IRUGO);
 MODULE_PARM_DESC(myri10ge_intr_coal_delay, "Interrupt coalescing delay");
 
+static int myri10ge_adapt_med_thresh = 8 * 1024 * 1024;
+module_param(myri10ge_adapt_med_thresh, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(myri10ge_adapt_med_thresh,
+		 "Low latency limit, in bytes per second");
+
+static int myri10ge_adapt_big_thresh = 256 * 1024 * 1024;
+module_param(myri10ge_adapt_big_thresh, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(myri10ge_adapt_big_thresh,
+		 "Bulk latency limit, in bytes per second");
+
 static int myri10ge_flow_control = 1;
 module_param(myri10ge_flow_control, int, S_IRUGO);
 MODULE_PARM_DESC(myri10ge_flow_control, "Pause parameter");
@@ -1054,6 +1075,9 @@
 		ss->tx.stop_queue = 0;
 	}
 
+	mgp->adapt_coal.usecs = -1;
+	mgp->adapt_coal.old_rx_bytes = 0;
+	mgp->adapt_coal.old_tx_bytes = 0;
 	status = myri10ge_update_mac_address(mgp, mgp->dev->dev_addr);
 	myri10ge_change_pause(mgp, mgp->pause);
 	myri10ge_set_multicast_list(mgp->dev);
@@ -1648,6 +1672,7 @@
 	struct myri10ge_priv *mgp = netdev_priv(netdev);
 
 	coal->rx_coalesce_usecs = mgp->intr_coal_delay;
+	coal->use_adaptive_rx_coalesce = mgp->adapt_coal.enabled;
 	return 0;
 }
 
@@ -1655,6 +1680,20 @@
 myri10ge_set_coalesce(struct net_device *netdev, struct ethtool_coalesce *coal)
 {
 	struct myri10ge_priv *mgp = netdev_priv(netdev);
+	struct myri10ge_adapt_intr_coal *adapt = &mgp->adapt_coal;
+
+	if (coal->use_adaptive_rx_coalesce != adapt->enabled) {
+		adapt->enabled = coal->use_adaptive_rx_coalesce;
+		if (adapt->enabled) {
+			adapt->big_usecs = mgp->intr_coal_delay;
+			adapt->timer.expires =
+			    jiffies + HZ / MYRI10GE_INTR_COAL_PERIOD;
+			if (mgp->running)
+				add_timer(&adapt->timer);
+		} else {
+			del_timer_sync(&adapt->timer);
+		}
+	}
 
 	mgp->intr_coal_delay = coal->rx_coalesce_usecs;
 	put_be32(htonl(mgp->intr_coal_delay), mgp->intr_coal_delay_ptr);
@@ -2515,6 +2554,11 @@
 	mgp->running = MYRI10GE_ETH_RUNNING;
 	mgp->watchdog_timer.expires = jiffies + myri10ge_watchdog_timeout * HZ;
 	add_timer(&mgp->watchdog_timer);
+	if (mgp->adapt_coal.enabled) {
+		mgp->adapt_coal.timer.expires =
+		    jiffies + HZ / MYRI10GE_INTR_COAL_PERIOD;
+		add_timer(&mgp->adapt_coal.timer);
+	}
 	netif_tx_wake_all_queues(dev);
 
 	return 0;
@@ -2548,6 +2592,7 @@
 		return 0;
 
 	del_timer_sync(&mgp->watchdog_timer);
+	del_timer_sync(&mgp->adapt_coal.timer);
 	mgp->running = MYRI10GE_ETH_STOPPING;
 	for (i = 0; i < mgp->num_slices; i++) {
 		napi_disable(&mgp->ss[i].napi);
@@ -3009,6 +3054,46 @@
 	return stats;
 }
 
+static void myri10ge_intr_coal_timer(unsigned long arg)
+{
+	struct myri10ge_priv *mgp = (struct myri10ge_priv *)arg;
+	struct net_device_stats *stats = &mgp->stats;
+	struct myri10ge_adapt_intr_coal *adapt = &mgp->adapt_coal;
+	unsigned long bytes_per_sec, bytes, usecs;
+	unsigned long tx_bytes, rx_bytes;
+
+	if (adapt->enabled == 0)
+		return;
+
+	/* snapshot stats */
+	(void)myri10ge_get_stats(mgp->dev);
+
+	tx_bytes = stats->tx_bytes;
+	rx_bytes = stats->rx_bytes;
+
+	/* calculate bytes since last snapshot */
+	bytes = tx_bytes - adapt->old_tx_bytes;
+	bytes += rx_bytes - adapt->old_rx_bytes;
+
+	/* store snapshot for next time */
+	adapt->old_tx_bytes = tx_bytes;
+	adapt->old_rx_bytes = rx_bytes;
+
+	bytes_per_sec = bytes * MYRI10GE_INTR_COAL_PERIOD;
+	if (bytes_per_sec < myri10ge_adapt_med_thresh)
+		usecs = 0;
+	else if (bytes_per_sec < myri10ge_adapt_big_thresh)
+		usecs = adapt->big_usecs / 5;
+	else
+		usecs = adapt->big_usecs;
+
+	if (adapt->usecs != usecs) {
+		adapt->usecs = usecs;
+		put_be32(htonl(usecs), mgp->intr_coal_delay_ptr);
+	}
+	mod_timer(&adapt->timer, jiffies + HZ / MYRI10GE_INTR_COAL_PERIOD);
+}
+
 static void myri10ge_set_multicast_list(struct net_device *dev)
 {
 	struct myri10ge_priv *mgp = netdev_priv(dev);
@@ -3969,6 +4054,8 @@
 	/* Setup the watchdog timer */
 	setup_timer(&mgp->watchdog_timer, myri10ge_watchdog_timer,
 		    (unsigned long)mgp);
+	setup_timer(&mgp->adapt_coal.timer, myri10ge_intr_coal_timer,
+		    (unsigned long)mgp);
 
 	spin_lock_init(&mgp->stats_lock);
 	SET_ETHTOOL_OPS(netdev, &myri10ge_ethtool_ops);



^ permalink raw reply

* Re: [PATCH] iproute2 add hoplimit parsing and update usage and documentation
From: Eric Dumazet @ 2009-10-06 17:14 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: netdev, Ori Finkelman, Andreas Henriksson, Stephen Hemminger
In-Reply-To: <1254836434.544524.27946.nullmailer@watson.codefidence.com>

Gilad Ben-Yossef a écrit :
> From: Gilad Ben-Yossef <gilad@codefidence.com>
> 
> - Parse and handle the hoplimit ip route option and add it to the usage 
>   line and documentation.
> 
> - Add the missing reordering ip route option to the usage line.
> 
> - Add documentation for initcwnd ip route option.
> 
> Tested by setting hoplimit and retreiving it via "show". 
> 
> Signed-off-by: Gilad Ben-Yossef <gilad@codefidence.com>
> [ported to HEAD, fixed a bug with hoplimit lock handling, added documentation]
> Signed-off-by: Ori Finkelman <ori@comsleep.com>
> Signed-off-by: Yony Amit <yony@comsleep.com>
> 

> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -13,6 +13,8 @@
>   *
>   * Rani Assaf <rani@magic.metawire.com> 980929:	resolve addresses
>   * Kunihiro Ishiguro <kunihiro@zebra.org> 001102: rtnh_ifindex was not initialized
> + * Gilad Ben-Yossef <gilad@codefidence.com> 091006: Add hoplimit option parsing 
> + * and correct usage

Well, git/web history is much better than this, you dont need to add this comment.

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

patch should be sent to iproute2 maintainer, Stephen Hemminger <shemminger@vyatta.com>

He might be too busy to catch netdev mails these days, I dont know :)

Thanks

^ permalink raw reply

* Re: query: TCPCT setsockopt kmalloc's
From: Eric Dumazet @ 2009-10-06 16:31 UTC (permalink / raw)
  To: William Allen Simpson; +Cc: netdev
In-Reply-To: <4ACB67EB.6080000@gmail.com>

William Allen Simpson a écrit :
> On both the client and server side, the setsockopt does a kmalloc().  Only
> once per connect on the client side, once per listen on the server side.

But if server has to generate a random data for a connection, you'll have to clone
the data to be able to give it at application request ?

> 
> However, after Miller's expressed concern, it seems possible to reorganize
> the code to do the kmalloc() before the lock_sock().  Does that mean
> that it
> should be GFP_KERNEL?  Or should it still be GFP_ATOMIC?

GFP_KERNEL is better in this context, it allows the requestor to sleep a bit if necessary.

> 
> [Not that anybody cares, but based on recent discussion on the list about
> internal kernel coding standards, I've changed Adam's old sizeof(struct)
> to sizeof(*tcvp).  Previously, I was trying to make as few changes as
> possible, thinking everything was already correct.]



> 
> ===
> new:
> 
> +        /* Allocate ancillary memory before locking.
> +         */
> +        if ((0 < tcd.tcpcd_used

Ah, could you please reverse all your conditions ?

Their form are unusual in linux kernel.

if (tcd.tcpcd_used > 0)


> ...
> +         && NULL == (tcvp = kmalloc(sizeof(*tcvp) + tcd.tcpcd_used,
> +                        GFP_KERNEL))) {
> +            return -ENOMEM;
> +        }
>
 
tcvp = kmalloc(sizeof(*tcvp) + tcd.tcpcd_used, GFP_KERNEL));
if (tcvp == NULL) ...


^ permalink raw reply

* Re: [PATCH 1/3] bonding: allow previous slave to be used when re-balancing traffic on tlb/alb interfaces
From: Andy Gospodarek @ 2009-10-06 16:27 UTC (permalink / raw)
  To: David Miller; +Cc: fubar, andy, netdev
In-Reply-To: <20091005.034333.04801045.davem@davemloft.net>

On Mon, Oct 05, 2009 at 03:43:33AM -0700, David Miller wrote:
> From: Jay Vosburgh <fubar@us.ibm.com>
> Date: Fri, 02 Oct 2009 18:13:57 -0700
> 
> > 	Andy, I've been doing some further testing with this patch, and
> > I'm seeing some panics that I believe are related to this patch.  It
> > appears that the last_slave isn't cleared (or isn't cleared soon enough)
> > when a slave is released, and concurrent transmit activity is getting
> > into alb_get_best_slave() and finding a last_slave pointer that is stale
> > (points to no slave currently on the slave list).
> 
> I'm holding off on these 3 bonding patches until this is resolved.
> --

Sounds good, Dave.

I was hoping to get this resolved yesterday, but got distracted.  I will
have some time today.


^ permalink raw reply

* Re: [PATCH] pasemi_mac: ethtool set settings support
From: Olof Johansson @ 2009-10-06 16:11 UTC (permalink / raw)
  To: Valentine Barshak; +Cc: linuxppc-dev, jgarzik, netdev
In-Reply-To: <20091005133124.GA11717@ru.mvista.com>

On Mon, Oct 05, 2009 at 05:31:24PM +0400, Valentine Barshak wrote:
> Add ethtool set settings to pasemi_mac_ethtool.
> 
> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>

Acked-by: Olof Johansson <olof@lixom.net>

^ permalink raw reply

* Re: [PATCH] pasemi_mac: ethtool get settings fix
From: Olof Johansson @ 2009-10-06 16:10 UTC (permalink / raw)
  To: Valentine Barshak; +Cc: linuxppc-dev, jgarzik, netdev
In-Reply-To: <20091005132756.GA11704@ru.mvista.com>

Weird, I see my address in the to: line but I never got a copy in my inbox.

On Mon, Oct 05, 2009 at 05:27:56PM +0400, Valentine Barshak wrote:
> Not all pasemi mac interfaces can have a phy attached.
> For example, XAUI has no phy and phydev is NULL for it.
> In this case ethtool get settings causes kernel crash.
> Fix it by returning -EOPNOTSUPP if there's no PHY attached.
> 
> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>

Acked-by: Olof Johansson <olof@lixom.net>

> ---
>  drivers/net/pasemi_mac_ethtool.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff -pruN linux-2.6.orig/drivers/net/pasemi_mac_ethtool.c linux-2.6/drivers/net/pasemi_mac_ethtool.c
> --- linux-2.6.orig/drivers/net/pasemi_mac_ethtool.c	2009-02-14 03:23:08.000000000 +0300
> +++ linux-2.6/drivers/net/pasemi_mac_ethtool.c	2009-10-05 16:21:52.000000000 +0400
> @@ -71,6 +71,9 @@ pasemi_mac_ethtool_get_settings(struct n
>  	struct pasemi_mac *mac = netdev_priv(netdev);
>  	struct phy_device *phydev = mac->phydev;
>  
> +	if (!phydev)
> +		return -EOPNOTSUPP;
> +
>  	return phy_ethtool_gset(phydev, cmd);
>  }
>  
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* query: TCPCT setsockopt kmalloc's
From: William Allen Simpson @ 2009-10-06 15:53 UTC (permalink / raw)
  To: netdev

On both the client and server side, the setsockopt does a kmalloc().  Only
once per connect on the client side, once per listen on the server side.

However, after Miller's expressed concern, it seems possible to reorganize
the code to do the kmalloc() before the lock_sock().  Does that mean that it
should be GFP_KERNEL?  Or should it still be GFP_ATOMIC?

[Not that anybody cares, but based on recent discussion on the list about
internal kernel coding standards, I've changed Adam's old sizeof(struct)
to sizeof(*tcvp).  Previously, I was trying to make as few changes as
possible, thinking everything was already correct.]

===
new:

+		/* Allocate ancillary memory before locking.
+		 */
+		if ((0 < tcd.tcpcd_used
...
+		 && NULL == (tcvp = kmalloc(sizeof(*tcvp) + tcd.tcpcd_used,
+					    GFP_KERNEL))) {
+			return -ENOMEM;
+		}
+
+		lock_sock(sk);
...
+		} else {
+			if (unlikely(NULL != tp->cookie_values)) {
+				kref_put(&tp->cookie_values->kref,
+					 tcp_cookie_values_release);
+			}
+			kref_init(&tcvp->kref);
...
+			tp->cookie_values = tcvp;
+		}
+
+		release_sock(sk);
+		return err;

===
old:

+		lock_sock(sk);
...
+		} else if (NULL != (tsdplp =
+				    kmalloc(sizeof(struct tcp_s_data_payload)
+					    + tcd.tcpcd_used,
+					    GFP_ATOMIC))) {
+			if (unlikely(tp->s_data_payload)) {
+				kref_put(&tp->s_data_payload->kref,
+					 tcp_s_data_payload_release);
+			}
+			kref_init(&tsdplp->kref);
...
+			tp->s_data_payload = tsdplp;
+		} else {
+			err = -ENOMEM;
+		}
+
+		release_sock(sk);
+		return err;

^ permalink raw reply

* Re: [PATCH] ipv4: arp_notify address list bug
From: Eric Dumazet @ 2009-10-06 15:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, Hannes Frederic Sowa, netdev
In-Reply-To: <20091006082913.54883824@nehalam>

Stephen Hemminger a écrit :
> 
> Okay, but I can't see that sending out one per address is going to be a big
> storm. Can't imagine user with 100's of addresses on same interface, but I guess
> it is possible.
> 

I saw some setups with hundred of ip addresses.

But they probably dont change MAC addresses very often :)

Who knows...

^ permalink raw reply

* Support of multiple default routes in Linux ?
From: Mallika Gautam @ 2009-10-06 15:37 UTC (permalink / raw)
  To: linux-net, netdev

Hi All,

I am working on 2.6.26 kernel. I am trying to setup Policy based
routing with multiple IP addresses in same subnet, with multiple
routing tables, each having its own default gateway. I have no route
in the 'Main' table. Problem is that I am not able to add default
route in each of the routing table.

I have 3 interfaces in the same subnet, sharing the gateway. I am
using 3 different routing tables for them. I need to add this gateway
to each of the routing tables with different interface associated with
the routing table.

Routing tables look like this -

eth0: 192.168.1.1/16
eth1: 192.168.1.2/16
eth2: 192.168.1.3/16

main table: empty

---- eth0tbl: routing table for eth0 ----
192.168.0.0/16 dev eth0

--- eth1tbl: routing table for eth1 ---
192.168.0.0/16 dev eth1

--- eth2tbl: routing table for eth2 ---
192.168.0.0/16 dev eth2

#ip rule
0: from all lookup local
2: from 192.168.1.1 iif lo lookup eth0tbl
3: from all to 192.168.1.1 iif eth0 lookup eth0tbl
4: from 192.168.1.2 iif lo lookup eth1tbl
5: from all to 192.168.1.2 iif eth1 lookup eth1tbl
6: from 192.168.1.3 iif lo lookup eth2tbl
7: from all to 192.168.1.3 iif eth2 lookup eth2tbl
32766: from all lookup main ----------> /* empty */
32767: from all lookup default ----------> /* empty */

when I give following command to add default route, it returns error -
#ip route add default via 192.168.254.254 dev eth0 table eth0tbl
RTNETLINK answers: No such process.

#ip route add default via 192.168.254.254 dev eth1 table eth1tbl
RTNETLINK answers: No such process.

#ip route add default via 192.168.254.254 dev eth2 table eth2tbl
RTNETLINK answers: No such process.

arp_announce, arp_ignore and arp_filter are all set to 1.
Is there a way this can be achieved? Is this available in any of the
later kernel versions? Any pointers would be of help.

Thanks

^ permalink raw reply

* Re: [PATCH] ipv4: arp_notify address list bug
From: Stephen Hemminger @ 2009-10-06 15:29 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, Hannes Frederic Sowa, netdev
In-Reply-To: <4ACAD393.5080909@gmail.com>

On Tue, 06 Oct 2009 07:20:19 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> 
> This fixes a bug with arp_notify.
> 
> If arp_notify is enabled, kernel will crash if address is changed
> and no IP address is assigned.
>   http://bugzilla.kernel.org/show_bug.cgi?id=14330
> 
> Reported-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  net/ipv4/devinet.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index e92f1fd..5df2f6a 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -1077,12 +1077,16 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
>  		ip_mc_up(in_dev);
>  		/* fall through */
>  	case NETDEV_CHANGEADDR:
> -		if (IN_DEV_ARP_NOTIFY(in_dev))
> -			arp_send(ARPOP_REQUEST, ETH_P_ARP,
> -				 in_dev->ifa_list->ifa_address,
> -				 dev,
> -				 in_dev->ifa_list->ifa_address,
> -				 NULL, dev->dev_addr, NULL);
> +		/* Send gratuitous ARP to notify of link change */
> +		if (IN_DEV_ARP_NOTIFY(in_dev)) {
> +			struct in_ifaddr *ifa = in_dev->ifa_list;
> +
> +			if (ifa)
> +				arp_send(ARPOP_REQUEST, ETH_P_ARP,
> +					 ifa->ifa_address, dev,
> +					 ifa->ifa_address, NULL,
> +					 dev->dev_addr, NULL);
> +		}
>  		break;
>  	case NETDEV_DOWN:
>  		ip_mc_down(in_dev);

Okay, but I can't see that sending out one per address is going to be a big
storm. Can't imagine user with 100's of addresses on same interface, but I guess
it is possible.

-- 

^ permalink raw reply

* Re: [PATCH] net: add support for STMicroelectronics Ethernet controllers.
From: Giuseppe CAVALLARO @ 2009-10-06 13:42 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Andy Fleming
In-Reply-To: <4ACB47B5.5050700@gmail.com>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Eric.

Eric Dumazet wrote:
> Giuseppe CAVALLARO a écrit :
> 
>>> Why call dev_kfree_skb_any() here ? From NAPI context it is overkill.
>> The logic behind this piece of code should be the same one adopted in
>> other drivers like gianfar, ucc_geth and mv643xx_eth. What am I missing?
> 
> Dont trust driver code too much, many of them are not upd2date.
> 
> gianfar disables irqs, so it calls dev_kfree_skb_any()
> 
> if (spin_trylock_irqsave(&priv->txlock, flags)) {
> 	tx_cleaned = gfar_clean_tx_ring(dev);
> 	spin_unlock_irqrestore(&priv->txlock, flags);
> }
> 
> but in your case,
> 
> stmmac_clean_tx() runs in sofirq mode, you can call dev_kfree_skb()/consume_skb()
> 
> Please check drivers/net/tg3.c, function tg3_tx() for a _good_ example.

Thanks, I'll fix it taking as example the tg3 d.d.

>>> static int stmmac_poll(struct napi_struct *napi, int budget)
>>> +{
>> [snip]
>>> +
>>> +	tx_cleaned = stmmac_clean_tx(dev);
>>> +
>>> +	work_done = stmmac_rx(dev, budget);
>>> +
>>>
>>>
>>> +	if (tx_cleaned)
>>> +		return budget;
>>>
>>> Why tx_cleaned is used here to exit early ?
>> I've found interesting the approach used in gianfar (see commit
>> 42199884594bc336c9185441cbed99a9324dab34).
>>
> 
> This looks buggy and not a clone of e1000 code, despite its Changelog claim.
> 
> e1000 code is OK, not gianfar.
> 
> static int e1000_clean(struct napi_struct *napi, int budget)
> {
> 	tx_clean_complete = e1000_clean_tx_irq(adapter, &adapter->tx_ring[0]);
> 
> 	adapter->clean_rx(adapter, &adapter->rx_ring[0], &work_done, budget);
> 
> 	if (!tx_clean_complete)
> 		work_done = budget; // we say budget is fully consumed to force another poll round
> 
> 	if (work_done < budget) {
> 		...
> 		napi_complete(napi);
> 		...
> 	}
> }
> 
> You can see e1000_clean_tx_irq() doesnt return "number of completed skbs", but a 
> boolean saying if more skb are still in tx queue "(count < tx_ring->count)"
> 
> This is because we want to check again tx queue before napi_complete(this_adapter)
> 
> 
> Your code forces a poll() round if at least *one* skb was completed, this is very strange.
> 

I'll fix this too.

I'm going to post a new patch for the stmmac as soon as I fix all these
points.

Many thanks.
Regards,
Peppe
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkrLSTQACgkQ2Xo3j31MSSIKgQCeL733CI46GAUhDk/safdFH2Wj
q98An09JpuvsNGb0bihuy5dEaAIDUIIV
=R6wJ
-----END PGP SIGNATURE-----

^ permalink raw reply

* [PATCH] iproute2 add hoplimit parsing and update usage and documentation
From: Gilad Ben-Yossef @ 2009-10-06 13:40 UTC (permalink / raw)
  To: netdev; +Cc: Ori Finkelman, Andreas Henriksson
In-Reply-To: <4ACB39F2.6080002@codefidence.com>

From: Gilad Ben-Yossef <gilad@codefidence.com>

- Parse and handle the hoplimit ip route option and add it to the usage 
  line and documentation.

- Add the missing reordering ip route option to the usage line.

- Add documentation for initcwnd ip route option.

Tested by setting hoplimit and retreiving it via "show". 

Signed-off-by: Gilad Ben-Yossef <gilad@codefidence.com>
[ported to HEAD, fixed a bug with hoplimit lock handling, added documentation]
Signed-off-by: Ori Finkelman <ori@comsleep.com>
Signed-off-by: Yony Amit <yony@comsleep.com>

---

Debian is carrying a patch with identical functionality for some 
time now it seems.

Parts of this patch carved out from original patch by Yony Amit 
and Ori Finkelman from Comsleep Ltd. with minor bug
fixing and HEAD port by me.

Additional documentation bits inspired by Debian patch with unknown
author provided by Andreas Henriksson <andreas@fatal.se>

diff --git a/doc/ip-cref.tex b/doc/ip-cref.tex
index bb4eb78..f81adc5 100644
--- a/doc/ip-cref.tex
+++ b/doc/ip-cref.tex
@@ -1324,7 +1324,17 @@ peers are allowed to send to us.
     If it is not given, Linux uses the value selected with \verb|sysctl|
     variable \verb|net/ipv4/tcp_reordering|.
 
+\item \verb|hoplimit NUMBER|
 
+--- [2.5.74+ only] Maximum number of hops on the path to this destination. 
+    The default is the value selected with the \verb|sysctl| variable
+    \verb|net/ipv4/ip_default_ttl|.
+
+\item \verb|initcwnd NUMBER|
+--- [2.5.70+ only] Initial congestion window size for connections to 
+    this destination. Actual window size is this value multiplied by the
+    MSS (``Maximal Segment Size'') for same connection. The default is
+    zero, meaning to use the values specified in~\cite{RFC2414}.
 
 \item \verb|nexthop NEXTHOP|
 
@@ -2653,6 +2663,9 @@ http://www.cisco.com/univercd/cc/td/doc/product/software/ios120.
 \bibitem{RFC-DHCP} R.~Droms.
 ``Dynamic Host Configuration Protocol.'', RFC-2131
 
+\bibitem{RFC2414}  M.~Allman, S.~Floyd, C.~Partridge.
+``Increasing TCP's Initial Window'', RFC-2414.
+
 \end{thebibliography}
 
 
diff --git a/ip/iproute.c b/ip/iproute.c
index bf0f31b..aeea93d 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -13,6 +13,8 @@
  *
  * Rani Assaf <rani@magic.metawire.com> 980929:	resolve addresses
  * Kunihiro Ishiguro <kunihiro@zebra.org> 001102: rtnh_ifindex was not initialized
+ * Gilad Ben-Yossef <gilad@codefidence.com> 091006: Add hoplimit option parsing 
+ * and correct usage
  */
 
 #include <stdio.h>
@@ -70,10 +72,10 @@ static void usage(void)
 	fprintf(stderr, "INFO_SPEC := NH OPTIONS FLAGS [ nexthop NH ]...\n");
 	fprintf(stderr, "NH := [ via ADDRESS ] [ dev STRING ] [ weight NUMBER ] NHFLAGS\n");
 	fprintf(stderr, "OPTIONS := FLAGS [ mtu NUMBER ] [ advmss NUMBER ]\n");
-	fprintf(stderr, "           [ rtt TIME ] [ rttvar TIME ]\n");
+	fprintf(stderr, "           [ rtt TIME ] [ rttvar TIME ] [reordering NUMBER ]\n");
 	fprintf(stderr, "           [ window NUMBER] [ cwnd NUMBER ] [ initcwnd NUMBER ]\n");
 	fprintf(stderr, "           [ ssthresh NUMBER ] [ realms REALM ] [ src ADDRESS ]\n");
-	fprintf(stderr, "           [ rto_min TIME ]\n");
+	fprintf(stderr, "           [ rto_min TIME ] [ hoplimit NUMBER ] \n");
 	fprintf(stderr, "TYPE := [ unicast | local | broadcast | multicast | throw |\n");
 	fprintf(stderr, "          unreachable | prohibit | blackhole | nat ]\n");
 	fprintf(stderr, "TABLE_ID := [ local | main | default | all | NUMBER ]\n");
@@ -768,6 +770,18 @@ int iproute_modify(int cmd, unsigned flags, int argc, char **argv)
 			if (get_unsigned(&mtu, *argv, 0))
 				invarg("\"mtu\" value is invalid\n", *argv);
 			rta_addattr32(mxrta, sizeof(mxbuf), RTAX_MTU, mtu);
+#ifdef RTAX_HOPLIMIT
+		} else if (strcmp(*argv, "hoplimit") == 0) {
+			unsigned hoplimit;
+			NEXT_ARG();
+			if (strcmp(*argv, "lock") == 0) {
+				mxlock |= (1<<RTAX_HOPLIMIT);
+				NEXT_ARG();
+			}
+			if (get_unsigned(&hoplimit, *argv, 0))
+				invarg("\"hoplimit\" value is invalid\n", *argv);
+			rta_addattr32(mxrta, sizeof(mxbuf), RTAX_HOPLIMIT, hoplimit);
+#endif
 #ifdef RTAX_ADVMSS
 		} else if (strcmp(*argv, "advmss") == 0) {
 			unsigned mss;

^ permalink raw reply related

* Re: [PATCH] net: add support for STMicroelectronics Ethernet controllers.
From: Eric Dumazet @ 2009-10-06 13:35 UTC (permalink / raw)
  To: Giuseppe CAVALLARO; +Cc: netdev, Andy Fleming
In-Reply-To: <4ACB3A42.9070600@st.com>

Giuseppe CAVALLARO a écrit :

>> Why call dev_kfree_skb_any() here ? From NAPI context it is overkill.
> 
> The logic behind this piece of code should be the same one adopted in
> other drivers like gianfar, ucc_geth and mv643xx_eth. What am I missing?

Dont trust driver code too much, many of them are not upd2date.

gianfar disables irqs, so it calls dev_kfree_skb_any()

if (spin_trylock_irqsave(&priv->txlock, flags)) {
	tx_cleaned = gfar_clean_tx_ring(dev);
	spin_unlock_irqrestore(&priv->txlock, flags);
}

but in your case,

stmmac_clean_tx() runs in sofirq mode, you can call dev_kfree_skb()/consume_skb()

Please check drivers/net/tg3.c, function tg3_tx() for a _good_ example.

> 
>> static int stmmac_poll(struct napi_struct *napi, int budget)
>> +{
> [snip]
>> +
>> +	tx_cleaned = stmmac_clean_tx(dev);
>> +
>> +	work_done = stmmac_rx(dev, budget);
>> +
>>
>>
>> +	if (tx_cleaned)
>> +		return budget;
>>
>> Why tx_cleaned is used here to exit early ?
> 
> I've found interesting the approach used in gianfar (see commit
> 42199884594bc336c9185441cbed99a9324dab34).
> 

This looks buggy and not a clone of e1000 code, despite its Changelog claim.

e1000 code is OK, not gianfar.

static int e1000_clean(struct napi_struct *napi, int budget)
{
	tx_clean_complete = e1000_clean_tx_irq(adapter, &adapter->tx_ring[0]);

	adapter->clean_rx(adapter, &adapter->rx_ring[0], &work_done, budget);

	if (!tx_clean_complete)
		work_done = budget; // we say budget is fully consumed to force another poll round

	if (work_done < budget) {
		...
		napi_complete(napi);
		...
	}
}

You can see e1000_clean_tx_irq() doesnt return "number of completed skbs", but a 
boolean saying if more skb are still in tx queue "(count < tx_ring->count)"

This is because we want to check again tx queue before napi_complete(this_adapter)


Your code forces a poll() round if at least *one* skb was completed, this is very strange.

^ permalink raw reply

* [PATCH] ethoc: limit the number of buffers to 128
From: Thomas Chou @ 2009-10-06 13:25 UTC (permalink / raw)
  To: netdev; +Cc: thierry.reding, Nios2 development list, linux-kernel, Thomas Chou

Only 128 buffer descriptors are supported in the core. Limit the
number in case we have more memory.

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 drivers/net/ethoc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 6d82dc6..34d0c69 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -662,8 +662,8 @@ static int ethoc_open(struct net_device *dev)
 	if (ret)
 		return ret;
 
-	/* calculate the number of TX/RX buffers */
-	num_bd = (dev->mem_end - dev->mem_start + 1) / ETHOC_BUFSIZ;
+	/* calculate the number of TX/RX buffers, maximum 128 supported */
+	num_bd = min(128, (dev->mem_end - dev->mem_start + 1) / ETHOC_BUFSIZ);
 	priv->num_tx = max(min_tx, num_bd / 4);
 	priv->num_rx = num_bd - priv->num_tx;
 	ethoc_write(priv, TX_BD_NUM, priv->num_tx);
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH] netxen: Fix Unlikely(x) > y
From: Roel Kluin @ 2009-10-06 13:23 UTC (permalink / raw)
  To: Dhananjay Phadke, netdev, Andrew Morton

The closing parenthesis was not on the right location.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
This was intended, I think?

diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index b5aa974..9b9eab1 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -1714,7 +1714,7 @@ netxen_nic_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 	/* 4 fragments per cmd des */
 	no_of_desc = (frag_count + 3) >> 2;
 
-	if (unlikely(no_of_desc + 2) > netxen_tx_avail(tx_ring)) {
+	if (unlikely(no_of_desc + 2 > netxen_tx_avail(tx_ring))) {
 		netif_stop_queue(netdev);
 		return NETDEV_TX_BUSY;
 	}

^ permalink raw reply related

* Re: [PATCH] net: add support for STMicroelectronics Ethernet controllers.
From: Giuseppe CAVALLARO @ 2009-10-06 12:38 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <4ACB1B95.4090701@gmail.com>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Eric,
many thanks for your prompt feedback.

Eric Dumazet wrote:
> Giuseppe CAVALLARO a écrit :
> 
>> +static int stmmac_sw_tso(struct stmmac_priv *priv, struct sk_buff *skb)
[snip]
> 
> So stmmac_sw_tso() calls stmmac_xmit() for each seg skb.
> 
> But stmmac_sw_tso() was called from stmmac_xmit(),
> with priv->tx_lock locked, so I suspect something is wrong.

Yes, you are right on this.
I'm going to remove it. Indeed, I observed no gain during my performance
tests. I had added it just to become familiar with this.
Note also that our chips do not have the segmentation offloading in Hw.
Thanks for this observation.

> Also please change various
> 
> +	mac = kmalloc(sizeof(const struct mac_device_info), GFP_KERNEL);
> +	memset(mac, 0, sizeof(struct mac_device_info));
> +
> 
> +	mac = kmalloc(sizeof(const struct mac_device_info), GFP_KERNEL);
> +	memset(mac, 0, sizeof(struct mac_device_info));
> +
> 
> to use kzalloc(), and of course, you should check kmalloc()/kzalloc() dont return NULL !
> 

Yes I'll do that too.

> Also :
> 
> +static int stmmac_clean_tx(struct net_device *dev)
[snip]
> +		if (skb != NULL) {
> +			/*
> +			 * If there's room in the queue (limit it to size)
> +			 * we add this skb back into the pool,
> +			 * if it's the right size.
> +			 */
> +			if ((skb_queue_len(&priv->rx_recycle) <
> +				priv->dma_rx_size) &&
> +				skb_recycle_check(skb, priv->dma_buf_sz))
> +				__skb_queue_head(&priv->rx_recycle, skb);
> +			else
> +				dev_kfree_skb_any(skb);
> 
> Why call dev_kfree_skb_any() here ? From NAPI context it is overkill.

The logic behind this piece of code should be the same one adopted in
other drivers like gianfar, ucc_geth and mv643xx_eth. What am I missing?

> static int stmmac_poll(struct napi_struct *napi, int budget)
> +{
[snip]
> +
> +	tx_cleaned = stmmac_clean_tx(dev);
> +
> +	work_done = stmmac_rx(dev, budget);
> +
> 
> 
> +	if (tx_cleaned)
> +		return budget;
> 
> Why tx_cleaned is used here to exit early ?

I've found interesting the approach used in gianfar (see commit
42199884594bc336c9185441cbed99a9324dab34).

Thanks again for the feedback.

Regards,
Peppe
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkrLOjkACgkQ2Xo3j31MSSIM5ACgrTLJgwO84ooKkcoEaNMZ5bcy
iBUAoK+q677OnyeCdeNndFq92AmwNPoa
=r7WK
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: [PATCH] iproute2 add hoplimit and reordering route options usage and parsing
From: Gilad Ben-Yossef @ 2009-10-06 12:37 UTC (permalink / raw)
  To: Andreas Henriksson; +Cc: netdev, ori
In-Reply-To: <20091006114142.GA11012@amd64.fatal.se>

Andreas Henriksson wrote:

> Hello!
>
> On Mon, Oct 05, 2009 at 10:54:21AM +0200, Gilad Ben-Yossef wrote:
>   
>> iproute2 git HEAD (spotted originally on 2.6.26, so it's probably not new) 
>> does  not parse the hoplimit route option when proccessing parameters, nor 
>> does it  print hoplimit and reordering options in the usage lines.  
>>     
>
> I'm happy you brought this up and know the author. The Debian package
> has been carrying this code, which is now in the form of a patch available
> here:
> http://git.debian.org/?p=collab-maint/pkg-iproute.git;a=blob;f=debian/patches/hoplimit.dpatch
>
> I didn't know who the original author was which is why I haven't submitted it,
> but it seems you know the history....
>   
Not exactly. I know the history of the patch *I sent* (and btw, it seems 
I got the author name wrong). I don't think it is related to the one 
Debian is carrying.
> The above patch also includes some minor documentation updates which you seem
> to be missing and might want to integrate in your version of the patch.
>   
All right. I will add those documentation changes as well and resend as 
version 2.
> Looking forward to be able to drop the patch in the debian package!
>
>
>
>   
I take it you ACK the patch as the Debian package maintainer?

Thanks!
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker & CTO
Codefidence Ltd.

Web:   http://codefidence.com
Cell:  +972-52-8260388
Skype: gilad_codefidence
Tel:   +972-8-9316883 ext. 201
Fax:   +972-8-9316884
Email: gilad@codefidence.com

Check out our Open Source technology and training blog - http://tuxology.net

	"Now the world has gone to bed
	 Darkness won't engulf my head
	 I can see by infra-red
	 How I hate the night."


^ permalink raw reply

* Re: Adding to linux-next?
From: Stephen Rothwell @ 2009-10-06 11:57 UTC (permalink / raw)
  To: Gregory Haskins
  Cc: linux-next, linux-kernel@vger.kernel.org, netdev, David Miller,
	alacrityvm-devel@lists.sourceforge.net
In-Reply-To: <4AC6339B.7000905@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 4054 bytes --]

Hi Greg,

Sorry for the slow response.

On Fri, 02 Oct 2009 13:08:43 -0400 Gregory Haskins <gregory.haskins@gmail.com> wrote:
>
> I am looking for some guidance on policy/procedure governing inclusion
> of a tree to linux-next.  For instance: Do I have to be arbitrarily
> invited (e.g. by some committee on LKML), or do I explicitly request
> consideration?  I tried to Google around for answers, and also found the
> linux-next wiki, but I was not getting any clear answers.

You just send me the location of your tree and ask for inclusion. You
should also mention who should be contacted if I have any problems with
this tree (I will assume yourself).

> I have these guest drivers to support IO on top of the AlacrityVM
> hypervisor:
> 
> http://lkml.org/lkml/2009/8/3/278
> 
> The comments have since died down.  I realize this can mean anything
> from "no objection" to "no interest" ;), but I assume the former unless
> someone pipes up.
> 
> I believe I addressed the review comments and received an Ack from the
> one maintainer of the tree that overlaps with the work (netdev/davem), here:
> 
> http://lkml.org/lkml/2009/8/3/505
> 
> Since the rest of the work doesn't really fall into any existing
> subsystem, and David conceded that the netdev overlap portion should
> carry elsewhere, I offer to fill this role myself from within the
> AlacrityVM tree itself.
> 
> As such, I have taken the driver series and created a new branch here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/ghaskins/alacrityvm/linux-2.6.git
> linux-next

I will add this tree from tomorrow unless someone screams.

> Unlike the original posting, I have excluded the final ethernet patch
> since I posted a v3 today (http://lkml.org/lkml/2009/10/2/239) that I
> would like to have David re-Ack before including.
> 
> Once the driver has been suitably approved by David, and if he still
> feels its ok to carry in a tree other than netdev, I will re-add it to
> the linux-next branch.

That sounds fine.

> Because I am not really sure of the policies for linux-next, let me
> state my intentions of this branch, since I am an unknown in the
> maintainership role:
> 
> I will only post patches to this branch that:
> 
> *) do not fall into an existing maintained subsystem category, unless
> the appropriate maintainer has relinquished the patch to carry in my tree.
> *) have previously been posted to LKML for suitable review.

Here is my boilerplate:

Thanks for adding your subsystem tree as a participant of linux-next.  As
you may know, this is not a judgment of your code.  The purpose of
linux-next is for integration testing and to lower the impact of
conflicts between subsystems in the next merge window. 

You will need to ensure that the patches/commits in your tree/series have
been:
     * submitted under GPL v2 (or later) and include the Contributor's
	Signed-off-by,
     * posted to the relevant mailing list,
     * reviewed by you (or another maintainer of your subsystem tree),
     * successfully unit tested, and 
     * destined for the current or next Linux merge window.

Basically, this should be just what you would send to Linus (or ask him
to fetch).  It is allowed to be rebased if you deem it necessary.

-- 
Cheers,
Stephen Rothwell 
sfr@canb.auug.org.au

Legal Stuff:
By participating in linux-next, your subsystem tree contributions are
public and will be included in the linux-next trees.  You may be sent
e-mail messages indicating errors or other issues when the
patches/commits from your subsystem tree are merged and tested in
linux-next.  These messages may also be cross-posted to the linux-next
mailing list, the linux-kernel mailing list, etc.  The linux-next tree
project and IBM (my employer) make no warranties regarding the linux-next
project, the testing procedures, the results, the e-mails, etc.  If you
don't agree to these ground rules, let me know and I'll remove your tree
from participation in linux-next.

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH] iproute2 add hoplimit and reordering route options usage and parsing
From: Andreas Henriksson @ 2009-10-06 11:41 UTC (permalink / raw)
  To: Gilad Ben-Yossef; +Cc: netdev, ori
In-Reply-To: <1254732861.015524.19779.nullmailer@watson.codefidence.com>

Hello!

On Mon, Oct 05, 2009 at 10:54:21AM +0200, Gilad Ben-Yossef wrote:
> iproute2 git HEAD (spotted originally on 2.6.26, so it's probably not new) 
> does  not parse the hoplimit route option when proccessing parameters, nor 
> does it  print hoplimit and reordering options in the usage lines.  

I'm happy you brought this up and know the author. The Debian package
has been carrying this code, which is now in the form of a patch available
here:
http://git.debian.org/?p=collab-maint/pkg-iproute.git;a=blob;f=debian/patches/hoplimit.dpatch

I didn't know who the original author was which is why I haven't submitted it,
but it seems you know the history....
The above patch also includes some minor documentation updates which you seem
to be missing and might want to integrate in your version of the patch.

Looking forward to be able to drop the patch in the debian package!

Regards,
Andreas Henriksson

^ permalink raw reply

* [PATCH] net/hamradio: fix test in receive()
From: Roel Kluin @ 2009-10-06 11:20 UTC (permalink / raw)
  To: netdev, davem, Andrew Morton

The negation makes it a bool before the comparison and hence it
will never evaluate to true.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
Was this intended?

diff --git a/drivers/net/hamradio/baycom_epp.c b/drivers/net/hamradio/baycom_epp.c
index 7bcaf7c..ee06a13 100644
--- a/drivers/net/hamradio/baycom_epp.c
+++ b/drivers/net/hamradio/baycom_epp.c
@@ -596,7 +596,8 @@ static int receive(struct net_device *dev, int cnt)
 						state = 0;
 
 					/* not flag received */
-					else if (!(bitstream & (0x1fe << j)) != (0x0fc << j)) {
+					else if ((bitstream & (0x1fe << j)) !=
+							(0x0fc << j)) {
 						if (state)
 							do_rxpacket(dev);
 						bc->hdlcrx.bufcnt = 0;

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox