Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 3/3] ptp: derive the device name from the parent device
From: Ben Hutchings @ 2012-09-21 18:23 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Keller, Jacob E, netdev@vger.kernel.org, David Miller,
	Kirsher, Jeffrey T, John Stultz, Vick, Matthew
In-Reply-To: <20120921181115.GE6129@netboy.at.omicron.at>

On Fri, 2012-09-21 at 20:11 +0200, Richard Cochran wrote:
> On Fri, Sep 21, 2012 at 06:40:58PM +0100, Ben Hutchings wrote:
> > On Fri, 2012-09-21 at 17:39 +0000, Keller, Jacob E wrote:
> > > With the device, it is possible to lookup the driver.
> > 
> > For PCI devices, yes.  But there is really no guarantee that device
> > names (as in dev_name()) are globally unique either.
> 
> Ben, I tried to implement your suggestion from September 7th.
> Don't you like it, or did I misunderstand you?

For programmatic purposes, setting the parent is the really useful
thing.

I think what I'm still missing from you is some explanation of what the
'clock name' is meant to be used as - a type name, a unique identifier,
a 'friendly name' for listing clocks in a user interface?

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* [PATCH v3] ipconfig: add nameserver IPs to kernel-parameter ip=
From: Christoph Fritz @ 2012-09-21 18:31 UTC (permalink / raw)
  To: David Miller
  Cc: rob, kuznet, j.weitzel, jmorris, yoshfuji, kaber, linux-doc,
	linux-kernel, netdev, hjk, daniel
In-Reply-To: <20120921.133759.1667263015470766968.davem@davemloft.net>

On small systems (e.g. embedded ones) IP addresses are often configured
by bootloaders and get assigned to kernel via parameter "ip=".  If set to
"ip=dhcp", even nameserver entries from DHCP daemons are handled. These
entries exported in /proc/net/pnp are commonly linked by /etc/resolv.conf.

To configure nameservers for networks without DHCP, this patch adds option
<dns0-ip> and <dns1-ip> to kernel-parameter 'ip='.

Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
Tested-by: Jan Weitzel <j.weitzel@phytec.de>
---
v2: - fix indent
v3: - fix docu: add new arguments to option "ip="
---
 Documentation/filesystems/nfs/nfsroot.txt |   10 ++++++-
 net/ipv4/ipconfig.c                       |   39 ++++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/Documentation/filesystems/nfs/nfsroot.txt b/Documentation/filesystems/nfs/nfsroot.txt
index ffdd9d8..2d66ed6 100644
--- a/Documentation/filesystems/nfs/nfsroot.txt
+++ b/Documentation/filesystems/nfs/nfsroot.txt
@@ -78,7 +78,8 @@ nfsroot=[<server-ip>:]<root-dir>[,<nfs-options>]
 			flags		= hard, nointr, noposix, cto, ac
 
 
-ip=<client-ip>:<server-ip>:<gw-ip>:<netmask>:<hostname>:<device>:<autoconf>
+ip=<client-ip>:<server-ip>:<gw-ip>:<netmask>:<hostname>:<device>:<autoconf>:
+   <dns0-ip>:<dns1-ip>
 
   This parameter tells the kernel how to configure IP addresses of devices
   and also how to set up the IP routing table. It was originally called
@@ -158,6 +159,13 @@ ip=<client-ip>:<server-ip>:<gw-ip>:<netmask>:<hostname>:<device>:<autoconf>
 
                 Default: any
 
+  <dns0-ip>	IP address of first nameserver.
+		Value gets exported by /proc/net/pnp which is often linked
+		on embedded systems by /etc/resolv.conf.
+
+  <dns1-ip>	IP address of secound nameserver.
+		Same as above.
+
 
 nfsrootdebug
 
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 67e8a6b..1c0e7e0 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -743,14 +743,22 @@ static void __init ic_bootp_init_ext(u8 *e)
 
 
 /*
- *  Initialize the DHCP/BOOTP mechanism.
+ *  Predefine Nameservers
  */
-static inline void __init ic_bootp_init(void)
+static inline void __init ic_nameservers_predef(void)
 {
 	int i;
 
 	for (i = 0; i < CONF_NAMESERVERS_MAX; i++)
 		ic_nameservers[i] = NONE;
+}
+
+/*
+ *  Initialize the DHCP/BOOTP mechanism.
+ */
+static inline void __init ic_bootp_init(void)
+{
+	ic_nameservers_predef();
 
 	dev_add_pack(&bootp_packet_type);
 }
@@ -1379,6 +1387,7 @@ static int __init ip_auto_config(void)
 	int retries = CONF_OPEN_RETRIES;
 #endif
 	int err;
+	unsigned int i;
 
 #ifdef CONFIG_PROC_FS
 	proc_net_fops_create(&init_net, "pnp", S_IRUGO, &pnp_seq_fops);
@@ -1499,7 +1508,15 @@ static int __init ip_auto_config(void)
 		&ic_servaddr, &root_server_addr, root_server_path);
 	if (ic_dev_mtu)
 		pr_cont(", mtu=%d", ic_dev_mtu);
-	pr_cont("\n");
+	for (i = 0; i < CONF_NAMESERVERS_MAX; i++)
+		if (ic_nameservers[i] != NONE) {
+			pr_info("     nameserver%u=%pI4",
+				i, &ic_nameservers[i]);
+			break;
+		}
+	for (i++; i < CONF_NAMESERVERS_MAX; i++)
+		if (ic_nameservers[i] != NONE)
+			pr_cont(", nameserver%u=%pI4\n", i, &ic_nameservers[i]);
 #endif /* !SILENT */
 
 	return 0;
@@ -1570,6 +1587,8 @@ static int __init ip_auto_config_setup(char *addrs)
 		return 1;
 	}
 
+	ic_nameservers_predef();
+
 	/* Parse string for static IP assignment.  */
 	ip = addrs;
 	while (ip && *ip) {
@@ -1613,6 +1632,20 @@ static int __init ip_auto_config_setup(char *addrs)
 					ic_enable = 0;
 				}
 				break;
+			case 7:
+				if (CONF_NAMESERVERS_MAX >= 1) {
+					ic_nameservers[0] = in_aton(ip);
+					if (ic_nameservers[0] == ANY)
+						ic_nameservers[0] = NONE;
+				}
+				break;
+			case 8:
+				if (CONF_NAMESERVERS_MAX >= 2) {
+					ic_nameservers[1] = in_aton(ip);
+					if (ic_nameservers[1] == ANY)
+						ic_nameservers[1] = NONE;
+				}
+				break;
 			}
 		}
 		ip = cp;
-- 
1.7.2.5




^ permalink raw reply related

* Re: [PATCH] tcp: sysctl for initial receive window
From: Jesper Dangaard Brouer @ 2012-09-21 18:32 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, netdev, nanditad
In-Reply-To: <20120921.135601.254379488076661898.davem@davemloft.net>

On Fri, 2012-09-21 at 13:56 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 21 Sep 2012 17:25:11 +0200
> 
> > On Fri, 2012-09-21 at 10:55 +0200, Jesper Dangaard Brouer wrote:
> >> Make it possible to adjust the TCP default initial advertised receive
> >> window, via sysctl /proc/sys/net/ipv4/tcp_init_recv_window.
> >> 
> >> The window size is this value multiplied by the MSS of the connection.
> >> The default value is (still) 10, as descibed in commit 356f039822b
> >> (TCP: increase default initial receive window.)
> >> 
> >> Allow minimum value of 1, but recommend against setting value below 2
> >> in the documentation.
> >> 
> >> Its possible to control/override this value per route table entry via
> >> the iproute2 option initrwnd.  Having the global default exported via
> >> sysctl, helps determine the default setting, and make is easier to
> >> adjust.
> > 
> > I was wondering why its not symmetric :
> > 
> > If we add a sysctl for initial receive window, we need another one for
> > initial send window ?
> 
> Unlike the routing configuration, this is susceptible to serious abuse.

Are you talking about, this patch for "tcp_init_recv_window" initial
advertised receive window?


> All it takes is for one jackass vendor to say that this should be set
> to 1,000 in in sysctl.conf when using their product.

I do see your point with jackass vendors.

But (for tcp_init_recv_window) its not a problem, because this is being
limited by tcp_rmem[1] (and div 2 default due to tcp_adv_win_scale), and
can/is further be limited by window clamping. (and we also cut it if
tcp_adv_win_scale > 14).


> Whereas setting it on a per-route basis forces the person doing it
> to actually consider that there might be ramifications that have to
> do with the paths on which you are making this adjustment.

As I mentioned above, this also requires some extra work and
consideration to make this go out of bound.

> I would only let this in if you hard limited the setting to it's
> current setting, 10.  So people could decrease it.

The would defeat the purpose of the patch.  Perhaps we could, allow a
sensible max... (but this max is already being controlled as described).


-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: mlx4: dropping multicast packets at promisc leave
From: Marcelo Ricardo Leitner @ 2012-09-21 18:32 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: netdev, Yevgeny Petrilin, Amir Vadai
In-Reply-To: <505B3088.7090908@redhat.com>

On 09/20/2012 12:04 PM, Marcelo Ricardo Leitner wrote:
> On 09/20/2012 10:21 AM, Or Gerlitz wrote:
>> On 20/09/2012 03:43, Marcelo Ricardo Leitner wrote:
>>> I have a report that our mlx4 driver (RHEL 6.3) is dropping multicast
>>> packets when NIC leaves promisc mode. It seems this is being cause due
>>> to the new steering mode that took place near by commit
>>> 1679200f91da6a054b06954c9bd3eeed29b6731f. As it seems, the new
>>> steering mode needs more commands/time to leave the promisc mode,
>>> which may be leading to packet drops.
>>
>> Marcelo,
>>
>> The commit you point on below 6d19993 "net/mlx4_en: Re-design multicast
>> attachments flow" makes sure to avoid
>> doing extra firmware comments and not leave a window in time where
>> "correct" addresses are not attached. Its hard to say what's the case on
>> that RHEL 6.3 system, it would be very helpful through if you manage to
>> reproduce the problem on an upstream kernel -- BTW you didn't say on
>
> Okay, I understand that the commit prevents a window. I may be missing
> something, but isn't there another one in there? Between:
> mlx4_SET_MCAST_FLTR MLX4_MCAST_DISABLE and
> mlx4_SET_MCAST_FLTR MLX4_MCAST_ENABLE
> because mlx4_multicast_promisc_remove() was called just before those.
> Otherwise I don't how is the NIC would be receiving multicast packets in
> there.
>
....
> And then I tried 3 additional patches applied at once:
> - 60d31c1475f2 "net/mlx4_core: Looking for promiscuous entries on the
> correct port"
> - f1f75f0 - mlx4: attach multicast with correct flag
> - Yes, this one wasn't in 2.6.32-279.el6.
> - 6d19993 - net/mlx4_en: Re-design multicast attachments flow
>
> And they still reported drops.

Hi, updates on this. In summary, I couldn't reproduce it when using 
upstream kernel, even on a machine which I've seen the drops.

Checking with customer, they could reproduce the issue once when using 
commit 6d19993 but cannot reproduce it anymore.

I also could reproduce it but it gets much harder to trigger when using 
commit 6d19993.

I could run some tests with net tree kernel (up to 8ea853fd) and I 
didn't see any drops so far. More bellow.

>>> It takes 300ms to perform the change there against my 600us. Hitting
>>> something like tcpdump -c 10 in a loop helps triggering it.
>>
>> Do you have any insight for this huge difference?
>
> No idea. Couldn't track it yet.

Now I might have. ksoftirqd/0 is being triggered when running 500Mbit 
multicast traffic via iperf and is hitting 90% CPU usage. Seems it is 
blocking the execution of mlx4_en_do_set_multicast() sometimes.

This happens at my reproducer only. It does not happen on the other 
server. Customer also doesn't see it, but see the delays. They just see 
mlx4_en thread running when running tcpdump in cycles, but that is expected.

Upstream (net tree) kernel doesn't do this, even at my reproducer box.

Simple perf record/report gave me:

# Overhead      Command      Shared Object     Symbol
# ........  ...........  .................     .....................
#
      98.16%  ksoftirqd/0  [kernel.kallsyms]  [k]
  fix_small_imbalance
              |
              --- fix_small_imbalance
                  find_busiest_group
                  rebalance_domains
                  run_rebalance_domains
                  __do_softirq
                  call_softirq
                  ksoftirqd
                  kthread
                  child_rip

I could see:
[60315.574258] mlx4_core 0000:01:00.0: remove_promisc_qp 483
[60315.579652] mlx4_core 0000:01:00.0: remove_promisc_qp 485

And that was:
  482         /*remove from list of promisc qps */
  483         mlx4_warn(dev, "%s %d\n", __FUNCTION__, __LINE__);
  484         list_del(&pqp->list);
  485         mlx4_warn(dev, "%s %d\n", __FUNCTION__, __LINE__);

That really shouldn't take that long, and it doesn't when it's idle. 
Might that weird behavior open some window where NIC is configured in a 
such way that it doesn't receive the packets?

But well, this may be causing the drops somewhere else. I still have to 
check that.

Many thanks,
Marcelo.

^ permalink raw reply

* Re: [PATCH 1/3 V2] phy/micrel: Implement support for KSZ8021
From: Marek Vasut @ 2012-09-21 18:33 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, david.choi, nobuhiro.iwamatsu.yj, fabio.estevam,
	shawn.guo
In-Reply-To: <20120921.141037.1710579690343556147.davem@davemloft.net>

Dear David Miller,

> From: Marek Vasut <marex@denx.de>
> Date: Fri, 21 Sep 2012 20:04:09 +0200
> 
> >> Rather, the goal is to properly line up function arguments with
> >> the first column after the openning parenthesis on the previous
> >> line.  Using TAB and SPACE characters, as needed.
> >> 
> >> > +	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause
> >> > +				| SUPPORTED_Asym_Pause),
> >> 
> >> This is similarly not styled properly.
> > 
> > This is copy-pasted from other entry. I'd hate to reformat the whole
> > file.
> 
> I'm not asking you to, just the new code you're adding.

I don't like it. What'd you say to me linting the whole file with checkpatch in 
a subsequent patch?

Best regards,
Marek Vasut

^ permalink raw reply

* Re: [PATCH net-next 3/3] ptp: derive the device name from the parent device
From: Richard Cochran @ 2012-09-21 18:40 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Keller, Jacob E, netdev@vger.kernel.org, David Miller,
	Kirsher, Jeffrey T, John Stultz, Vick, Matthew
In-Reply-To: <1348251783.2521.42.camel@bwh-desktop.uk.solarflarecom.com>

On Fri, Sep 21, 2012 at 07:23:03PM +0100, Ben Hutchings wrote:
> 
> I think what I'm still missing from you is some explanation of what the
> 'clock name' is meant to be used as - a type name, a unique identifier,
> a 'friendly name' for listing clocks in a user interface?

The original idea was a type/friendly kind of thing.

Imagine you are the admin of some random box that should run PTP. You
do 'cat /sys/class/ptp/ptp0/clock_name' and see "gianfar clock". Then
you think to yourself, "Excellent, I know that one, it works great,
and I can even get a PPS output."

But if you saw "IXP46X timer" then you would think, "Forget it, this
will never work."

That was the idea.

But before the ethtool thing came along, people started putting MAC
addresses in that string, and it continued even after the ethtool
method appeared. I wanted to correct the MAC abuses, but then I
thought you were concurring with putting some kind of semi-unique
device ID there.

I really don't care too much about the clock_name anyhow, because I
always know my own hardware. I don't mind changing it from type-string
or friendly-name to device ID, if people think that is more useful.

Thanks,
Richard

^ permalink raw reply

* multicast, interfaces, kernel 3.0+...
From: Michael Tokarev @ 2012-09-21 18:46 UTC (permalink / raw)
  To: netdev

Hello.

We found some, well, interesting behavour of kernels
3.0 and later, while 2.6.32 (previous long-stable
series) worked fine.  I'm not sure when it "broke",
since this is a production machine and we've difficult
time diagnosing it, and the app causing it is, well,
large.

The short story.  A big java app uses multicast group
to register one component and find it later.

The machine in question has 3 active network interfaces:
usual lo, eth0, and virtual (tap, pointopoint) tinc.
Tinc interface is marked as "multicast off".

When the app starts on 2.6.32 kernel, netstat -g shows
that multicast group on 2 interfaces: lo and eth0, but
not on tinc, which is sort of expected:

$ netstat -g
IPv6/IPv4 Group Memberships
Interface       RefCnt Group
--------------- ------ ---------------------
lo              4      228.5.6.7
lo              1      all-systems.mcast.net
eth0            4      228.5.6.7
eth0            1      all-systems.mcast.net
tinc            1      all-systems.mcast.net


But when the same app (actually the same userspace) is
booted on the same machine but on 3.0+ kernel, the same
multicast group is registered also on 2 interfaces, but
this time these are lo (as before) and tinc, but not eth0:

$ netstat -g
IPv6/IPv4 Group Memberships
Interface       RefCnt Group
--------------- ------ ---------------------
lo              4      228.5.6.7
lo              1      all-systems.mcast.net
eth0            1      all-systems.mcast.net
tinc            4      228.5.6.7
tinc            1      all-systems.mcast.net

Now, on 3.0+ kernel, parts of this app can't find each
other.  The "client" tries to send a datagram packet
to this address, 228.5.6.7, but receives no reply.

On 2.6.32 kernel, when eth0 is used instead of tinc,
it all works as expected.

Now, my knowlege of this multicast stuff is very limited
(reading about it now), so I don't really know what it
all means.  At least the fact that it somehow registers
tinc (which is multicast-off!) is already somewhat strange.
I tried removing this multicast setting from this iface,
but that didn't help.  I also tried enabling multicast on
lo (which was disabled!) and disabling it on others, but
that didn't help either.

According to strace, the app does not try to change iface
group membership, it does bind of a udp socket to 0.0.0.0:port,
and uses SOL_IP, IP_ADD_MEMBERSHIP to add this socket to a
multicast group.

Note: there's just ONE machine involved, and two applications
running on it.

Why with 3.0+, the non-multicast "tinc" interface is shown
as a member of 228.5.6.7 group, but not eth0 which actually
*is* multicast?

For the record, this "big java app" is Oracle reports server.
I've no idea why they use multicast to find two components
of one thing running on the same machine, and does not provide
any usable unicast solution...

Thanks!

/mjt

^ permalink raw reply

* Re: [PATCH] tcp: sysctl for initial receive window
From: David Miller @ 2012-09-21 18:48 UTC (permalink / raw)
  To: brouer; +Cc: eric.dumazet, netdev, nanditad
In-Reply-To: <1348252326.3103.90.camel@localhost>

From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Fri, 21 Sep 2012 20:32:06 +0200

> On Fri, 2012-09-21 at 13:56 -0400, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Fri, 21 Sep 2012 17:25:11 +0200
>> 
>> I would only let this in if you hard limited the setting to it's
>> current setting, 10.  So people could decrease it.
> 
> The would defeat the purpose of the patch.  Perhaps we could, allow a
> sensible max... (but this max is already being controlled as described).

Any new max which is truly sensible, could be the new default, and we
would apply the same amount of vetting for such a thing.

^ permalink raw reply

* Re: [PATCH 1/3 V2] phy/micrel: Implement support for KSZ8021
From: David Miller @ 2012-09-21 18:48 UTC (permalink / raw)
  To: marex; +Cc: netdev, david.choi, nobuhiro.iwamatsu.yj, fabio.estevam,
	shawn.guo
In-Reply-To: <201209212033.12738.marex@denx.de>

From: Marek Vasut <marex@denx.de>
Date: Fri, 21 Sep 2012 20:33:12 +0200

> I don't like it.

Then I'm not applying your patches.

^ permalink raw reply

* Re: [PATCH 1/3 V2] phy/micrel: Implement support for KSZ8021
From: Marek Vasut @ 2012-09-21 18:50 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, david.choi, nobuhiro.iwamatsu.yj, fabio.estevam,
	shawn.guo
In-Reply-To: <20120921.144840.730495056587914350.davem@davemloft.net>

Dear David Miller,

> From: Marek Vasut <marex@denx.de>
> Date: Fri, 21 Sep 2012 20:33:12 +0200
> 
> > I don't like it.
> 
> Then I'm not applying your patches.

What about the other part of my offer ?

Best regards,
Marek Vasut

^ permalink raw reply

* Re: [PATCH v3] ipconfig: add nameserver IPs to kernel-parameter ip=
From: David Miller @ 2012-09-21 18:51 UTC (permalink / raw)
  To: chf.fritz
  Cc: rob, kuznet, j.weitzel, jmorris, yoshfuji, kaber, linux-doc,
	linux-kernel, netdev, hjk, daniel
In-Reply-To: <1348252279.12456.2.camel@mars>

From: Christoph Fritz <chf.fritz@googlemail.com>
Date: Fri, 21 Sep 2012 20:31:19 +0200

> On small systems (e.g. embedded ones) IP addresses are often configured
> by bootloaders and get assigned to kernel via parameter "ip=".  If set to
> "ip=dhcp", even nameserver entries from DHCP daemons are handled. These
> entries exported in /proc/net/pnp are commonly linked by /etc/resolv.conf.
> 
> To configure nameservers for networks without DHCP, this patch adds option
> <dns0-ip> and <dns1-ip> to kernel-parameter 'ip='.
> 
> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
> Tested-by: Jan Weitzel <j.weitzel@phytec.de>

Applied to net-next, thanks.

^ permalink raw reply

* Re: [PATCH] Xen backend support for paged out grant targets V4.
From: Konrad Rzeszutek Wilk @ 2012-09-21 18:52 UTC (permalink / raw)
  To: Andres Lagar-Cavilla
  Cc: Ian Campbell, Andres Lagar-Cavilla, xen-devel, David Vrabel,
	David Miller, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org
In-Reply-To: <5B5132A4-93B2-41D0-B1A6-048810565DB5@gridcentric.ca>

On Mon, Sep 17, 2012 at 05:29:24AM -0400, Andres Lagar-Cavilla wrote:
> On Sep 17, 2012, at 4:17 AM, Ian Campbell wrote:
> 
> > (I think I forgot to hit send on this on Friday, sorry. Also
> > s/xen.lists.org/lists.xen.org in the CC line…)
> I'm on a roll here…
> 
> > 
> > On Fri, 2012-09-14 at 15:26 +0100, Andres Lagar-Cavilla wrote:
> >> Since Xen-4.2, hvm domains may have portions of their memory paged out. When a
> >> foreign domain (such as dom0) attempts to map these frames, the map will
> >> initially fail. The hypervisor returns a suitable errno, and kicks an
> >> asynchronous page-in operation carried out by a helper. The foreign domain is
> >> expected to retry the mapping operation until it eventually succeeds. The
> >> foreign domain is not put to sleep because itself could be the one running the
> >> pager assist (typical scenario for dom0).
> >> 
> >> This patch adds support for this mechanism for backend drivers using grant
> >> mapping and copying operations. Specifically, this covers the blkback and
> >> gntdev drivers (which map foregin grants), and the netback driver (which copies
> > 
> >                           foreign
> > 
> >> foreign grants).
> >> 
> >> * Add a retry method for grants that fail with GNTST_eagain (i.e. because the
> >>  target foregin frame is paged out).
> > 
> >          foreign
> > 
> >> * Insert hooks with appropriate wrappers in the aforementioned drivers.
> >> 
> >> The retry loop is only invoked if the grant operation status is GNTST_eagain.
> >> It guarantees to leave a new status code different from GNTST_eagain. Any other
> >> status code results in identical code execution as before.
> >> 
> >> The retry loop performs 256 attempts with increasing time intervals through a
> >> 32 second period. It uses msleep to yield while waiting for the next retry.
> > [...]
> >> Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > Since this is more about grant tables than netback this should probably
> > go via Konrad rather than Dave, is that OK with you Dave?
> 
> If that is the case hopefully Konrad can deal with the two typos? Otherwise happy to re-spin the patch.

So with this patch when I launch an PVHVM guest on Xen 4.1 I get this
in the initial domain and the guest is crashed:

[  261.927218] privcmd_fault: vma=ffff88002a31dce8 7f4edc095000-7f4edc195000, pgoff=c8, uv=00007f4edc15d000

guest config:
> more /mnt/lab/latest/hvm.xm 
kernel = "/usr/lib/xen/boot/hvmloader"
builder='hvm'
memory=1024
#maxmem=1024
maxvcpus = 2
serial='pty'
vcpus = 2
disk = [ 'file:/mnt/lab/latest/root_image.iso,hdc:cdrom,r']
boot="dn"
#vif = [ 'type=ioemu,model=e1000,mac=00:0F:4B:00:00:71, bridge=switch' ]
vif = [ 'type=netfront, bridge=switch' ]
#vfb = [ 'vnc=1, vnclisten=0.0.0.0 ,vncunused=1']
vnc=1
vnclisten="0.0.0.0"
usb=1
xen_platform_pci=1

^ permalink raw reply

* Re: [PATCH 1/3 V2] phy/micrel: Implement support for KSZ8021
From: David Miller @ 2012-09-21 18:54 UTC (permalink / raw)
  To: marex; +Cc: netdev, david.choi, nobuhiro.iwamatsu.yj, fabio.estevam,
	shawn.guo
In-Reply-To: <201209212050.46319.marex@denx.de>

From: Marek Vasut <marex@denx.de>
Date: Fri, 21 Sep 2012 20:50:46 +0200

> Dear David Miller,
> 
>> From: Marek Vasut <marex@denx.de>
>> Date: Fri, 21 Sep 2012 20:33:12 +0200
>> 
>> > I don't like it.
>> 
>> Then I'm not applying your patches.
> 
> What about the other part of my offer ?

I read it, and if it were relevant to this discussion I would
have quoted it.

Can you please just fix up your patches exactly how I have asked you
to so that I can get these bug fixes into the 'net' tree?

This is completely rediculous, nobody else pushes back when I make
simple coding style correction requests for their changes like you
are.

You're behavior is self defeating, it's causing your work to not be
useful and to not propagate to the very place where people can benefit
from it the most.

^ permalink raw reply

* Re: linux-next: manual merge of the net-next tree with the net tree
From: David Miller @ 2012-09-21 18:58 UTC (permalink / raw)
  To: sfr; +Cc: netdev, linux-next, linux-kernel, bjorn
In-Reply-To: <20120921113009.29a296b6405b96b4d3c05f84@canb.auug.org.au>

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 21 Sep 2012 11:30:09 +1000

> Today's linux-next merge of the net-next tree got a conflict in
> drivers/net/usb/qmi_wwan.c between commit 9db273f45686 ("net:
> qmi_wwan: adding Huawei E367, ZTE MF683 and Pantech P4200") from the
> net tree and commit bd877e489126 ("net: qmi_wwan: use a single bind
> function for all device types") from the net-next tree.
> 
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).

Thanks Stephen, this should be resolved over the weekend as I plan
to do a merge during this time.

^ permalink raw reply

* Re: [PATCH] net/stmmac: Use clk_prepare_enable and clk_disable_unprepare
From: David Miller @ 2012-09-21 19:00 UTC (permalink / raw)
  To: viresh.kumar; +Cc: sr, netdev, peppe.cavallaro, spear-devel
In-Reply-To: <CAKohpomw8u5wgtWAukfqH7t3iwge+jyFxBY4bv6AT2u5c_=arA@mail.gmail.com>

From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Fri, 21 Sep 2012 16:51:28 +0530

> On 21 September 2012 16:36, Stefan Roese <sr@denx.de> wrote:
>> This patch fixes an issue introduced by commit ID 6a81c26f
>> [net/stmmac: remove conditional compilation of clk code], which
>> switched from the internal stmmac_clk_{en}{dis}able calls to
>> clk_{en}{dis}able. By this, calling clk_prepare and clk_unprepare
>> was removed.
>>
>> clk_{un}prepare is mandatory for platforms using common clock framework.
>> Since these drivers are used by SPEAr platform, which supports common
>> clock framework, add clk_{un}prepare() support for them. Otherwise
>> the clocks are not correctly en-/disabled and ethernet support doesn't
>> work.
> 
> I can't believe i have done this. :)
> 
> IIRC, when i wrote this code prepare/unprepare weren't there. And by the
> time my code got merged, they were. And this mistake was missed there.
> 
> Thanks for fixing it.
> 
> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

Applied, thanks.

^ permalink raw reply

* Re: [patch net-next] team: send port changed when added
From: David Miller @ 2012-09-21 19:01 UTC (permalink / raw)
  To: jiri; +Cc: netdev
In-Reply-To: <1348228319-11453-1-git-send-email-jiri@resnulli.us>

From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 21 Sep 2012 13:51:59 +0200

> On some hw, link is not up during adding iface to team. That causes event
> not being sent to userspace and that may cause confusion.
> Fix this bug by sending port changed event once it's added to team.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>

Applied, thanks a lot Jiri.

^ permalink raw reply

* Re: [PATCH net-next 3/3] ptp: derive the device name from the parent device
From: Ben Hutchings @ 2012-09-21 19:01 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Keller, Jacob E, netdev@vger.kernel.org, David Miller,
	Kirsher, Jeffrey T, John Stultz, Vick, Matthew
In-Reply-To: <20120921184035.GF6129@netboy.at.omicron.at>

On Fri, 2012-09-21 at 20:40 +0200, Richard Cochran wrote:
> On Fri, Sep 21, 2012 at 07:23:03PM +0100, Ben Hutchings wrote:
> > 
> > I think what I'm still missing from you is some explanation of what the
> > 'clock name' is meant to be used as - a type name, a unique identifier,
> > a 'friendly name' for listing clocks in a user interface?
> 
> The original idea was a type/friendly kind of thing.
> 
> Imagine you are the admin of some random box that should run PTP. You
> do 'cat /sys/class/ptp/ptp0/clock_name' and see "gianfar clock". Then
> you think to yourself, "Excellent, I know that one, it works great,
> and I can even get a PPS output."
> 
> But if you saw "IXP46X timer" then you would think, "Forget it, this
> will never work."
> 
> That was the idea.
> 
> But before the ethtool thing came along, people started putting MAC
> addresses in that string, and it continued even after the ethtool
> method appeared. I wanted to correct the MAC abuses, but then I
> thought you were concurring with putting some kind of semi-unique
> device ID there.

I was confused about whether it was actually meant to be unique.

The ethtool command is useful but setting the parent device may be even
more useful, e.g. you will be able to write udev rules for PHC devices
based on the parent device's identity.

> I really don't care too much about the clock_name anyhow, because I
> always know my own hardware. I don't mind changing it from type-string
> or friendly-name to device ID, if people think that is more useful.

I don't mind either, just so long as the rule is either (1) implemented
in the PTP core code or (2) made very clear to driver writers.

The lack of a parent for the IXP device makes (1) hard to do well.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH net-next] be2net: Ignore spurious UE indication from NIC
From: David Miller @ 2012-09-21 19:04 UTC (permalink / raw)
  To: ajit.khaparde; +Cc: netdev
In-Reply-To: <20120921163620.GA6147@akhaparde-VBox>

From: Ajit Khaparde <ajit.khaparde@emulex.com>
Date: Fri, 21 Sep 2012 11:36:20 -0500

> Ignore spurious UE indication seen on some platforms.
> Consider the error as un-recoverable only when the bits
> stay high during second sampling.
> 
> Signed-off-by: Ajit Khaparde <ajit.khaparde@emulex.com>

Treating uncorrectable errors as spurious seems like an invitation
for hard to track down data corruption to me.

You'll need to come up with a more sophisticated test for
spurious other than "happens more than once" before I'm willing
to subject the entire world to this kind of potential problem.

^ permalink raw reply

* Re: [PATCH 1/3 V2] phy/micrel: Implement support for KSZ8021
From: Marek Vasut @ 2012-09-21 19:06 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, david.choi, nobuhiro.iwamatsu.yj, fabio.estevam,
	shawn.guo
In-Reply-To: <20120921.145405.646315503417542108.davem@davemloft.net>

Dear David Miller,

> From: Marek Vasut <marex@denx.de>
> Date: Fri, 21 Sep 2012 20:50:46 +0200
> 
> > Dear David Miller,
> > 
> >> From: Marek Vasut <marex@denx.de>
> >> Date: Fri, 21 Sep 2012 20:33:12 +0200
> >> 
> >> > I don't like it.
> >> 
> >> Then I'm not applying your patches.
> > 
> > What about the other part of my offer ?
> 
> I read it, and if it were relevant to this discussion I would
> have quoted it.
> 
> Can you please just fix up your patches exactly how I have asked you
> to so that I can get these bug fixes into the 'net' tree?

Sure, as you wish.

> This is completely rediculous, nobody else pushes back when I make
> simple coding style correction requests for their changes like you
> are.

You know, youth and all ... I was under the impression the patches shall be 
checkpatch clean. But you got me there quite well, something must be wrong with 
my precommit hook.

> You're behavior is self defeating, it's causing your work to not be
> useful and to not propagate to the very place where people can benefit
> from it the most.

Hey, this hurt. Anyway, about the checkpatch cleanup of the file, will that be 
welcome (afterwards I fix the patchset and repost)? Now that you pointed out 
there's more trouble in the file that is.

Best regards,
Marek Vasut

^ permalink raw reply

* Re: [PATCH 1/3 V2] phy/micrel: Implement support for KSZ8021
From: David Miller @ 2012-09-21 19:11 UTC (permalink / raw)
  To: marex; +Cc: netdev, david.choi, nobuhiro.iwamatsu.yj, fabio.estevam,
	shawn.guo
In-Reply-To: <201209212106.53069.marex@denx.de>

From: Marek Vasut <marex@denx.de>
Date: Fri, 21 Sep 2012 21:06:52 +0200

> You know, youth and all ... I was under the impression the patches shall be 
> checkpatch clean. But you got me there quite well, something must be wrong with 
> my precommit hook.

checkpatch is not a panacea, and it is in particular not an automaton
that one uses without using any human judgement at all.

In particular, checkpatch does not enforce the comment style we use in
the networking code nor several other conventions that we use which
are slightly different from the rest of the tree.

Therefore strick checkpatch conformance is never appropriate.

> Anyway, about the checkpatch cleanup of the file, will that be 
> welcome (afterwards I fix the patchset and repost)?

See above, strict checkpatch cleanups, especially those done in
a completely automaton style with zero human judgment involved,
are not welcome.

^ permalink raw reply

* Re: [PATCH 1/3 V2] phy/micrel: Implement support for KSZ8021
From: Marek Vasut @ 2012-09-21 19:19 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, david.choi, nobuhiro.iwamatsu.yj, fabio.estevam,
	shawn.guo
In-Reply-To: <20120921.151104.718226045755546422.davem@davemloft.net>

Dear David Miller,

> From: Marek Vasut <marex@denx.de>
> Date: Fri, 21 Sep 2012 21:06:52 +0200
> 
> > You know, youth and all ... I was under the impression the patches shall
> > be checkpatch clean. But you got me there quite well, something must be
> > wrong with my precommit hook.
> 
> checkpatch is not a panacea, and it is in particular not an automaton
> that one uses without using any human judgement at all.
> 
> In particular, checkpatch does not enforce the comment style we use in
> the networking code nor several other conventions that we use which
> are slightly different from the rest of the tree.
> 
> Therefore strick checkpatch conformance is never appropriate.

Understood.

> > Anyway, about the checkpatch cleanup of the file, will that be
> > welcome (afterwards I fix the patchset and repost)?
> 
> See above, strict checkpatch cleanups, especially those done in
> a completely automaton style with zero human judgment involved,
> are not welcome.

I meant the .features field ... it seems that the | at the following line 
appears in other PHY drivers as well though. Lets leave it at that anyway.

Best regards,
Marek Vasut

^ permalink raw reply

* RE: bnx2x: link detected up at startup even when it should be down
From: Dmitry Kravkov @ 2012-09-21 19:23 UTC (permalink / raw)
  To: Jean-Michel Hautbois
  Cc: netdev, Barak Witkowski, Eilon Greenstein, davem@davemloft.net
In-Reply-To: <CAL8zT=i_eV3SjuBgdWrBPaE-CoBBVh5QJD8jg-YMoQHDuA4AOQ@mail.gmail.com>

Hi Jean,

Thank you for the info

> -----Original Message-----
> From: Jean-Michel Hautbois [mailto:jhautbois@gmail.com]
> Sent: Friday, September 21, 2012 9:04 AM
> To: Dmitry Kravkov
> Cc: netdev; Barak Witkowski; Eilon Greenstein; davem@davemloft.net
> Subject: Re: bnx2x: link detected up at startup even when it should be down
> 
> I already did it twice. I think this is FW related and not only the
> commit adding afex support. It may have solved a link issue (I am just
> guessing, based on experiments) ?

FW replaced in this commit does not deal with link at all,

One is involved in link management is displayed using 'ethtool -i' - it comes with a card  and driver independent. 

Is device configured for MF by the switch?
Can you please share lspci output?

Other thing that can help in analysis is msglvl 0x4 for both situations

Thanks a lot



^ permalink raw reply

* [PATCH] sunbmac: Remove unused local variable.
From: David Miller @ 2012-09-21 19:23 UTC (permalink / raw)
  To: netdev


Commit eb716c54b1c71ad28ab20461bff831bd481066c4 ("sunbmac: remove
unnecessary setting of skb->dev") caused the local varible 'dev'
in bigmac_init_rings to become unused.  And now the compiler
warns about it.

Signed-off-by: David S. Miller <davem@davemloft.net>
---

Noticed while doing some sparc test builds, committed to net-next

 drivers/net/ethernet/sun/sunbmac.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/sun/sunbmac.c b/drivers/net/ethernet/sun/sunbmac.c
index 967fe8c..c9c977b 100644
--- a/drivers/net/ethernet/sun/sunbmac.c
+++ b/drivers/net/ethernet/sun/sunbmac.c
@@ -212,7 +212,6 @@ static void bigmac_clean_rings(struct bigmac *bp)
 static void bigmac_init_rings(struct bigmac *bp, int from_irq)
 {
 	struct bmac_init_block *bb = bp->bmac_block;
-	struct net_device *dev = bp->dev;
 	int i;
 	gfp_t gfp_flags = GFP_KERNEL;
 
-- 
1.7.11.4

^ permalink raw reply related

* Re: New commands to configure IOV features
From: Yinghai Lu @ 2012-09-21 19:23 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Rose, Gregory V, Bjorn Helgaas, Yuval Mintz, davem@davemloft.net,
	netdev@vger.kernel.org, Ariel Elior, Eilon Greenstein, linux-pci
In-Reply-To: <1348248958.2521.23.camel@bwh-desktop.uk.solarflarecom.com>

On Fri, Sep 21, 2012 at 10:35 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
> On Thu, 2012-09-20 at 22:50 -0700, Yinghai Lu wrote:
> [...]
>> Index: linux-2.6/include/linux/pci.h
>> ===================================================================
>> --- linux-2.6.orig/include/linux/pci.h
>> +++ linux-2.6/include/linux/pci.h
>> @@ -663,6 +663,7 @@ struct pci_driver {
>>         const struct pci_device_id *id_table;   /* must be non-NULL for probe to be called */
>>         int  (*probe)  (struct pci_dev *dev, const struct pci_device_id *id);   /* New device inserted */
>>         void (*remove) (struct pci_dev *dev);   /* Device removed (NULL if not a hot-plug capable driver) */
>> +       void (*set_max_vfs) (struct pci_dev *dev); /* enable sriov */
>>         int  (*suspend) (struct pci_dev *dev, pm_message_t state);      /* Device suspended */
>>         int  (*suspend_late) (struct pci_dev *dev, pm_message_t state);
>>         int  (*resume_early) (struct pci_dev *dev);
>
> Not sure about this; the driver may fail to enable those VFs and it
> would be nice to be able to report that failure directly.
>
> That said, this is 'max_vfs' (a limit) and if the driver fails to set up
> all the VFs then the *limit* may still be changed.
>
>> Subject: [PATCH] PCI: Add max_vfs in sysfs per pci device where supports
>>
>> driver later need to check dev->max_vfs in /sys
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> [...]
>> +static ssize_t
>> +max_vfs_store(struct device *dev, struct device_attribute *attr,
>> +                const char *buf, size_t count)
>> +{
>> +       unsigned long val;
>> +       struct pci_dev *pdev = to_pci_dev(dev);
>> +
>> +       if (strict_strtoul(buf, 0, &val) < 0)
>> +               return -EINVAL;
>> +
>> +       pdev->max_vfs = val;
>> +
>> +       if (pdev->is_added) {
>
> No locking required here?

should be removed.

>
>> +               int err;
>> +               err = device_schedule_callback(dev, max_vfs_callback);
>
> Any particular reason this should be a callback?

no, just copied that from bus rescan.

>
> [...]
>> Index: linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> +++ linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> @@ -129,13 +129,6 @@ static struct notifier_block dca_notifie
>>  };
>>  #endif
>>
>> -#ifdef CONFIG_PCI_IOV
>> -static unsigned int max_vfs;
>> -module_param(max_vfs, uint, 0);
>> -MODULE_PARM_DESC(max_vfs,
>> -                "Maximum number of virtual functions to allocate per
>> physical function - default is zero and maximum value is 63");
>> -#endif /* CONFIG_PCI_IOV */
>> -
>>  static unsigned int allow_unsupported_sfp;
>>  module_param(allow_unsupported_sfp, uint, 0);
>>  MODULE_PARM_DESC(allow_unsupported_sfp,
>> @@ -4528,7 +4521,7 @@ static int __devinit ixgbe_sw_init(struc
>>  #ifdef CONFIG_PCI_IOV
>>         /* assign number of SR-IOV VFs */
>>         if (hw->mac.type != ixgbe_mac_82598EB)
>> -               adapter->num_vfs = (max_vfs > 63) ? 0 : max_vfs;
>> +               adapter->num_vfs = (pdev->max_vfs > 63) ? 0 : pdev->max_vfs;
>
> We are trying to make all SR-IOV capable drivers work the same, so this
> weird limiting behaviour should not be retained.
>
> So I think the correct assignment is:
>                 adapter->num_vfs = min(pdev->max_vfs, 63);

this will treat >63 as 63. old one treat > 63 as 0 aka disabled.

looks your version is more reasonable...

>
>>  #endif
>>         /* enable itr by default in dynamic mode */
>> @@ -7249,8 +7242,9 @@ static int __devinit ixgbe_probe(struct
>>
>>  #ifdef CONFIG_PCI_IOV
>>         ixgbe_enable_sriov(adapter, ii);
>> -
>>  #endif
>> +       adapter->ixgbe_info = ii;
>> +
>>         netdev->features = NETIF_F_SG |
>>                            NETIF_F_IP_CSUM |
>>                            NETIF_F_IPV6_CSUM |
>> @@ -7720,11 +7714,42 @@ static const struct pci_error_handlers i
>>         .resume = ixgbe_io_resume,
>>  };
>>
>> +static void ixgbe_set_max_vfs(struct pci_dev *pdev)
>> +{
>> +#ifdef CONFIG_PCI_IOV
>> +       struct ixgbe_adapter *adapter = pci_get_drvdata(pdev);
>> +       struct net_device *netdev = adapter->netdev;
>> +       struct ixgbe_hw *hw = &adapter->hw;
>> +       int num_vfs = 0;
>> +
>> +       /* assign number of SR-IOV VFs */
>> +       if (hw->mac.type != ixgbe_mac_82598EB)
>> +               num_vfs = (pdev->max_vfs > 63) ? 0 : pdev->max_vfs;
>> +
>> +       /* no change */
>> +       if (adapter->num_vfs == num_vfs)
>> +               return;
>> +
>> +       if (!num_vfs) {
>> +               /* disable sriov */
>> +               ixgbe_disable_sriov(adapter);
>> +               adapter->num_vfs = 0;
>> +       } else if (!adapter->num_vfs && num_vfs) {
>> +               /* enable sriov */
>> +               adapter->num_vfs = num_vfs;
>> +               ixgbe_enable_sriov(adapter, adapter->ixgbe_info);
>> +       } else {
>> +               /* increase or decrease */
>
> Indeed, increase or decrease is not supported either in our PCI API or
> in the SR-IOV spec.  I think I would prefer the PCI core to filter out
> unsupported changes (i.e. not call set_max_vfs and maybe report an
> error), but I'm not sure about that.

should still call set_max_vfs, and let it set finally valid max_vfs.

pci driver should know better which max_vfs is better for exact ...

-Yinghai

^ permalink raw reply

* Re: [PATCH v3] ipconfig: add nameserver IPs to kernel-parameter ip=
From: Christoph Fritz @ 2012-09-21 19:28 UTC (permalink / raw)
  To: David Miller
  Cc: rob, kuznet, j.weitzel, jmorris, yoshfuji, kaber, linux-doc,
	linux-kernel, netdev, hjk, daniel
In-Reply-To: <20120921.145144.14955032475453606.davem@davemloft.net>

On Fri, 2012-09-21 at 14:51 -0400, David Miller wrote:
> From: Christoph Fritz <chf.fritz@googlemail.com>
> Date: Fri, 21 Sep 2012 20:31:19 +0200
> 
> > On small systems (e.g. embedded ones) IP addresses are often configured
> > by bootloaders and get assigned to kernel via parameter "ip=".  If set to
> > "ip=dhcp", even nameserver entries from DHCP daemons are handled. These
> > entries exported in /proc/net/pnp are commonly linked by /etc/resolv.conf.
> > 
> > To configure nameservers for networks without DHCP, this patch adds option
> > <dns0-ip> and <dns1-ip> to kernel-parameter 'ip='.
> > 
> > Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
> > Tested-by: Jan Weitzel <j.weitzel@phytec.de>
> 
> Applied to net-next, thanks.

Thanks a lot for your reviews.

 -- Christoph


^ permalink raw reply


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