* Re: How to use gretap with bridge?
From: Herbert Xu @ 2009-10-30 15:51 UTC (permalink / raw)
To: Neulinger, Nathan, David S. Miller; +Cc: shemminger, netdev
In-Reply-To: <846C5B546E47494CBBD796CA8CA1617EA3B43C@MST-VMAIL1.srv.mst.edu>
Neulinger, Nathan <nneul@mst.edu> wrote:
>
> The above change fixes it for me, but I'm no expert on this chunk of
> code. (Perhaps it it shouldn't set dev_addr at all?)
OK, it was a stupid mistake on my part. I added a netdev ops
struct for tap but didn't actually use it! Please let us know
whether this patch fixes the problem.
gre: Fix dev_addr clobbering for gretap
Nathan Neulinger noticed that gretap devices get their MAC address
from the local IP address, which results in invalid MAC addresses
half of the time.
This is because gretap is still using the tunnel netdev ops rather
than the correct tap netdev ops struct.
This patch also fixes changelink to not clobber the MAC address
for the gretap case.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 41ada99..1433338 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1464,7 +1464,7 @@ static void ipgre_tap_setup(struct net_device *dev)
ether_setup(dev);
- dev->netdev_ops = &ipgre_netdev_ops;
+ dev->netdev_ops = &ipgre_tap_netdev_ops;
dev->destructor = free_netdev;
dev->iflink = 0;
@@ -1525,25 +1525,29 @@ static int ipgre_changelink(struct net_device *dev, struct nlattr *tb[],
if (t->dev != dev)
return -EEXIST;
} else {
- unsigned nflags = 0;
-
t = nt;
- if (ipv4_is_multicast(p.iph.daddr))
- nflags = IFF_BROADCAST;
- else if (p.iph.daddr)
- nflags = IFF_POINTOPOINT;
+ if (dev->type != ARPHRD_ETHER) {
+ unsigned nflags = 0;
- if ((dev->flags ^ nflags) &
- (IFF_POINTOPOINT | IFF_BROADCAST))
- return -EINVAL;
+ if (ipv4_is_multicast(p.iph.daddr))
+ nflags = IFF_BROADCAST;
+ else if (p.iph.daddr)
+ nflags = IFF_POINTOPOINT;
+
+ if ((dev->flags ^ nflags) &
+ (IFF_POINTOPOINT | IFF_BROADCAST))
+ return -EINVAL;
+ }
ipgre_tunnel_unlink(ign, t);
t->parms.iph.saddr = p.iph.saddr;
t->parms.iph.daddr = p.iph.daddr;
t->parms.i_key = p.i_key;
- memcpy(dev->dev_addr, &p.iph.saddr, 4);
- memcpy(dev->broadcast, &p.iph.daddr, 4);
+ if (dev->type != ARPHRD_ETHER) {
+ memcpy(dev->dev_addr, &p.iph.saddr, 4);
+ memcpy(dev->broadcast, &p.iph.daddr, 4);
+ }
ipgre_tunnel_link(ign, t);
netdev_state_change(dev);
}
^ permalink raw reply related
* Re: [PATCH] sky2: set carrier off in probe
From: Herbert Xu @ 2009-10-30 15:54 UTC (permalink / raw)
To: David Miller; +Cc: bphilips, shemminger, netdev
In-Reply-To: <20091029.213816.35637194.davem@davemloft.net>
David Miller <davem@davemloft.net> wrote:
>
> I agree that we should report something consistent before interface
> up and 'no' is probably the best.
>
> I remember fixing something similar in other drivers a few months
> ago.
Can't we do this in one spot rather than having every driver
duplicate this?
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] udev: create empty regular files to represent net interfaces
From: dann frazier @ 2009-10-30 16:08 UTC (permalink / raw)
To: Narendra_K
Cc: greg, Matt_Domsch, kay.sievers, linux-hotplug, netdev,
Jordan_Hargrave, Charles_Rose, bhutchings
In-Reply-To: <EDA0A4495861324DA2618B4C45DCB3EE589682@blrx3m08.blr.amer.dell.com>
On Fri, Oct 30, 2009 at 08:43:44PM +0530, Narendra_K@Dell.com wrote:
>
> >> This way the kernel has only one name, and so does userspace, and
> >> everyone is happy.
> >
> >There are two issues, which really seem distinct to me.
> >
> >Users expect eth0 to map to first-onboard-nic. That's an
> >installer issue (since the BIOS can already export this info)
> >and I agree that if we want to "fix" that, we should fix it there.
> >
>
> I agree that installers have to be fixed in the sense that they can be
> told to find the right interface. But, they expect determinism and
> depend on "eth0 to map to first-onboard-nic". Installer is one of the
> applications that is affected by this and needs user intervention, if it
> is not told about the right interface. I discussed installer as it is so
> much part of a user experience.
Right, but couldn't the installer do the work of scanning the SMBIOS
to figure out which nics are onboard, and reorder the 'eth*' names
such that these are first? This state could then be written out as
udev rules so that they persist across reboots.
> But the real issue is "eth0 does not map to first-onboard-nic" always
> and applications expecting this would break in data center environments.
> Both the solutions proposed provide a way to overcome it without
> introducing state.
>
> With regards,
> Narendra K
>
--
dann frazier
^ permalink raw reply
* Re: [PATCH 1/3] net: TCP thin-stream detection
From: William Allen Simpson @ 2009-10-30 16:13 UTC (permalink / raw)
To: Linux Kernel Network Developers
Cc: apetlund, Ilpo Järvinen, Arnd Hannemann,
linux-kernel@vger.kernel.org, shemminger@vyatta.com,
davem@davemloft.net, Christian Samsel
In-Reply-To: <52481f5b4edfdbd96dcde46aed403fcf.squirrel@webmail.uio.no>
apetlund@simula.no wrote:
> I share the opinion that the linear timeouts should be limited, and back
> off exponentially after the limit, as Eric suggested. I believe this will
> be a sufficient safety-valve for the black-hole scenario, although I would
> like to run some tests.
>
> As I wrote to Arnd, there are many similarities with the EFR approach and
> what our patch does. The largest difference is that the thin-stream
> patterns are identified as an indication of time dependent/interactive
> apps. This is the reason why the proposed patch does not try to keep an
> inflated cwnd open, but only focuses on the cases of few packets in
> flight. The target is time-dependent/interactive applications, and as such
> we don't want a generally enabled mechanism, but want to give the option
> of enabling it only in the cases where they are most needed (in contrast
> to a generally enabled "automagically" triggered EFR).
>
> Below is a link to a table presenting some of the applications that we
> have traced and analysed the packet interarrival times of:
>
> http://folk.uio.no/apetlund/lktmp/thin_apps_table.pdf
>
> We were surprised to see how many cases of "thin-stream" traffic patterns
> were indicative of time-dependent/interactive apps.
>
I'm finding it hard to follow 3 threads, for the 3 parts of the patch.
As I mentioned in one of these threads, I've plenty of experience with
designing and implementing protocols for gaming. And it seems to me that
you're making changes to the entire TCP stack to make up for shortcomings
in the implementor's design. Yet, these changes require application
implementors to set a sockopt that's only available in Linux. Unlikely,
as they probably don't even keep track of such things....
There are other efforts in this area, they've been mentioned.
I'm new to this list, so I'm not entirely sure that protocol design is
regularly discussed here. But I'd prefer that the discussion moved to
one of the lists that's dedicated to such protocol design and testing.
I've already suggested the end-to-end interest list, where you'll find many
of us with a strong interest in this topic.
List-Subscribe: <http://mailman.postel.org/mailman/listinfo/end2end-interest>,
<mailto:end2end-interest-request@postel.org?subject=subscribe>
The IETF has two related working groups:
tcpm -- tcp modifications
tsvwg -- general transport, including sctp modifications
Without further ado, just count me as opposed at this time.
^ permalink raw reply
* Re: Connection tracking and vlan
From: Eric Dumazet @ 2009-10-30 16:19 UTC (permalink / raw)
To: Herbert Xu; +Cc: Adayadil Thomas, netdev, Patrick McHardy
In-Reply-To: <20091030154639.GA8197@gondor.apana.org.au>
Herbert Xu a écrit :
> On Fri, Oct 30, 2009 at 04:31:50PM +0100, Eric Dumazet wrote:
>> Same thing if you have two interfaces, eth0 & eth1 : IP conntrack tuples dont
>> include interface name/index
>
> Indeed, but imagine what happens when eth0 is the LAN and eth1 is
> the wild wild Internet. Do you really want their packets to mix?
>
No, Abayadi needs firewall rules (or RPF), before entering conntrack.
Allowing spoofed packets to come from wild Internet would be...
interesting in many aspects.
And since some setups use several links to LAN, several links to
Internet, its user policy decisions.
^ permalink raw reply
* Re: [PATCH] udev: create empty regular files to represent net interfaces
From: Bryan Kadzban @ 2009-10-30 16:22 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Kay Sievers, Matt Domsch, dann frazier, linux-hotplug, Narendra_K,
netdev, Jordan_Hargrave, Charles_Rose, Ben Hutchings
In-Reply-To: <4AEA997D.7070205@suse.de>
[-- Attachment #1: Type: text/plain, Size: 1523 bytes --]
Hannes Reinecke wrote:
> And to throw in some bit of useless information;
Stirring the pot a bit myself with this message...
> The one reason I didn't to this was that a network interface is _not_
> a file, but rather an abstract type which is known only internally in
> the kernel (ie the one exemption from the 'everything is a file' UNIX
> rule).
Why? Why not make it a file? I've heard rumors of other Unix-like
systems that do exactly that, FWIW.
(Yes, I'm joking. Well, maybe half-joking... It'd be nice, but I don't
expect it to happen.)
> When I were to design this, I would be implementing network interface
> _aliases_, so that a network interface could be accessed either by
> name or by alias. This mechanism can then be managed by udev, much
> like we (ie SUSE) is using it nowadays to assign the network
> interface numbers.
The problem with that, if I understand what you're suggesting, is the
value of IFNAMSIZ, and the fact that it can't be made any bigger. All
your aliases have to be IFNAMSIZ characters or less. And that's too
short to be able to embed the same level of information as we get for
e.g. disks. It's *barely* long enough to fit "mac-" plus 12 hex digits
(for the MAC address), but is completely incapable of holding a USB bus
path, for instance.
(Not that you'd want to use path persistence for USB devices. But it is
possible that you'd want it for some other setup, at which point it
becomes impossible to use the same rules for USB.)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]
^ permalink raw reply
* Re: Connection tracking and vlan
From: Patrick McHardy @ 2009-10-30 16:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Herbert Xu, Adayadil Thomas, netdev
In-Reply-To: <4AEB06E6.6020206@gmail.com>
Eric Dumazet wrote:
> Herbert Xu a écrit :
>> Adayadil Thomas <adayadil.thomas@gmail.com> wrote:
>>> If two connections have same 5 tuple, src ip, dst ip, src port, dst
>>> port, protocol(tcp/udp)
>>> but on different vlans (different vlan id), does the conntrack separate these ?
>> Probably not. Patrick, can you confirm this?
Yes, you are right.
> Very strange, this question about vlan looks like discussion we had
> yesterday (or the day before...) about interfaces (versus packet defragmentation)
Indeed, we did have that discussion a couple of years ago. IIRC
Rusty also suggested to add the interface to the defragmentation
key to avoid having fragments from different interfaces being
reassembled since iptables interface matches will only match on
the interface of the first fragment.
^ permalink raw reply
* Re: Connection tracking and vlan
From: Patrick McHardy @ 2009-10-30 16:27 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Herbert Xu, Adayadil Thomas, netdev
In-Reply-To: <4AEB1212.6010905@gmail.com>
Eric Dumazet wrote:
> Herbert Xu a écrit :
>> On Fri, Oct 30, 2009 at 04:31:50PM +0100, Eric Dumazet wrote:
>>> Same thing if you have two interfaces, eth0 & eth1 : IP conntrack tuples dont
>>> include interface name/index
>> Indeed, but imagine what happens when eth0 is the LAN and eth1 is
>> the wild wild Internet. Do you really want their packets to mix?
>>
>
> No, Abayadi needs firewall rules (or RPF), before entering conntrack.
>
> Allowing spoofed packets to come from wild Internet would be...
> interesting in many aspects.
>
> And since some setups use several links to LAN, several links to
> Internet, its user policy decisions.
Correct, users need to take care of this manually.
^ permalink raw reply
* Re: [PATCH] udev: create empty regular files to represent net interfaces
From: Stephen Hemminger @ 2009-10-30 16:34 UTC (permalink / raw)
To: Bryan Kadzban
Cc: Hannes Reinecke, Kay Sievers, Matt Domsch, dann frazier,
linux-hotplug, Narendra_K, netdev, Jordan_Hargrave, Charles_Rose,
Ben Hutchings
In-Reply-To: <4AEB12CC.1070300@kadzban.is-a-geek.net>
On Fri, 30 Oct 2009 09:22:36 -0700
Bryan Kadzban <bryan@kadzban.is-a-geek.net> wrote:
> Hannes Reinecke wrote:
> > And to throw in some bit of useless information;
>
> Stirring the pot a bit myself with this message...
>
> > The one reason I didn't to this was that a network interface is _not_
> > a file, but rather an abstract type which is known only internally in
> > the kernel (ie the one exemption from the 'everything is a file' UNIX
> > rule).
>
> Why? Why not make it a file? I've heard rumors of other Unix-like
> systems that do exactly that, FWIW.
>
> (Yes, I'm joking. Well, maybe half-joking... It'd be nice, but I don't
> expect it to happen.)
>
> > When I were to design this, I would be implementing network interface
> > _aliases_, so that a network interface could be accessed either by
> > name or by alias. This mechanism can then be managed by udev, much
> > like we (ie SUSE) is using it nowadays to assign the network
> > interface numbers.
>
> The problem with that, if I understand what you're suggesting, is the
> value of IFNAMSIZ, and the fact that it can't be made any bigger. All
> your aliases have to be IFNAMSIZ characters or less. And that's too
> short to be able to embed the same level of information as we get for
> e.g. disks. It's *barely* long enough to fit "mac-" plus 12 hex digits
> (for the MAC address), but is completely incapable of holding a USB bus
> path, for instance.
>
> (Not that you'd want to use path persistence for USB devices. But it is
> possible that you'd want it for some other setup, at which point it
> becomes impossible to use the same rules for USB.)
Not a big fan of multiple names, it is the wrong solution to the human
question of "what is eth0 really?". Router o/s use description field
for that kind of information.
I added ifalias to provide place to put user visible description information.
It is in latest kernels via sysfs and iproute utilities. Also plan to add
support for it in net-snmp.
--
^ permalink raw reply
* Re: How to use gretap with bridge?
From: Stephen Hemminger @ 2009-10-30 16:39 UTC (permalink / raw)
To: Herbert Xu; +Cc: Neulinger, Nathan, David S. Miller, netdev
In-Reply-To: <20091030155148.GA8283@gondor.apana.org.au>
On Fri, 30 Oct 2009 11:51:48 -0400
Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Neulinger, Nathan <nneul@mst.edu> wrote:
> >
> > The above change fixes it for me, but I'm no expert on this chunk of
> > code. (Perhaps it it shouldn't set dev_addr at all?)
>
> OK, it was a stupid mistake on my part. I added a netdev ops
> struct for tap but didn't actually use it! Please let us know
> whether this patch fixes the problem.
>
> gre: Fix dev_addr clobbering for gretap
>
> Nathan Neulinger noticed that gretap devices get their MAC address
> from the local IP address, which results in invalid MAC addresses
> half of the time.
>
> This is because gretap is still using the tunnel netdev ops rather
> than the correct tap netdev ops struct.
>
> This patch also fixes changelink to not clobber the MAC address
> for the gretap case.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> Thanks,
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
--
^ permalink raw reply
* RE: [PATCH] udev: create empty regular files to represent netinterfaces
From: Narendra_K @ 2009-10-30 16:53 UTC (permalink / raw)
To: dannf
Cc: greg, Matt_Domsch, kay.sievers, linux-hotplug, netdev,
Jordan_Hargrave, Charles_Rose, bhutchings
In-Reply-To: <20091030160845.GA4547@lackof.org>
>> >There are two issues, which really seem distinct to me.
>> >
>> >Users expect eth0 to map to first-onboard-nic. That's an installer
>> >issue (since the BIOS can already export this info) and I
>agree that
>> >if we want to "fix" that, we should fix it there.
>> >
>>
>> I agree that installers have to be fixed in the sense that
>they can be
>> told to find the right interface. But, they expect determinism and
>> depend on "eth0 to map to first-onboard-nic". Installer is
>one of the
>> applications that is affected by this and needs user
>intervention, if
>> it is not told about the right interface. I discussed
>installer as it
>> is so much part of a user experience.
>
>Right, but couldn't the installer do the work of scanning the
>SMBIOS to figure out which nics are onboard, and reorder the
>'eth*' names such that these are first? This state could then
>be written out as udev rules so that they persist across reboots.
>
I suppose, with udev loading modules, the rules generated at runtime
could run into the problem of duplicate names, if names are reordered in
the kernel namespace. (I.e the eth* namespace). Hence idea of an
alternate namespace.
With regards,
Narendra K
^ permalink raw reply
* Re: Connection tracking and vlan
From: Herbert Xu @ 2009-10-30 16:55 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Adayadil Thomas, netdev, Patrick McHardy
In-Reply-To: <4AEB1212.6010905@gmail.com>
On Fri, Oct 30, 2009 at 05:19:30PM +0100, Eric Dumazet wrote:
>
> No, Abayadi needs firewall rules (or RPF), before entering conntrack.
rp_filter doesn't help because routing occurs after conntrack,
so IP fragments from the untrusted side may have already been
folded into a fragment from the trusted side.
Yes netfilter rules pre-conntrack would help but I bet there
are a lot folks out there who don't have them.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] udev: create empty regular files to represent netinterfaces
From: dann frazier @ 2009-10-30 17:05 UTC (permalink / raw)
To: Narendra_K
Cc: greg, Matt_Domsch, kay.sievers, linux-hotplug, netdev,
Jordan_Hargrave, Charles_Rose, bhutchings
In-Reply-To: <EDA0A4495861324DA2618B4C45DCB3EE589684@blrx3m08.blr.amer.dell.com>
On Fri, Oct 30, 2009 at 10:23:57PM +0530, Narendra_K@Dell.com wrote:
>
> >> >There are two issues, which really seem distinct to me.
> >> >
> >> >Users expect eth0 to map to first-onboard-nic. That's an installer
> >> >issue (since the BIOS can already export this info) and I
> >agree that
> >> >if we want to "fix" that, we should fix it there.
> >> >
> >>
> >> I agree that installers have to be fixed in the sense that
> >they can be
> >> told to find the right interface. But, they expect determinism and
> >> depend on "eth0 to map to first-onboard-nic". Installer is
> >one of the
> >> applications that is affected by this and needs user
> >intervention, if
> >> it is not told about the right interface. I discussed
> >installer as it
> >> is so much part of a user experience.
> >
> >Right, but couldn't the installer do the work of scanning the
> >SMBIOS to figure out which nics are onboard, and reorder the
> >'eth*' names such that these are first? This state could then
> >be written out as udev rules so that they persist across reboots.
> >
>
> I suppose, with udev loading modules, the rules generated at runtime
> could run into the problem of duplicate names, if names are reordered in
> the kernel namespace. (I.e the eth* namespace). Hence idea of an
> alternate namespace.
I don't see a risk of duplicate names - after all drivers are loaded,
the installer can take the names enumerated by the kernel, figure out
what it thinks a preferrable order is (i.e. by querying SMBIOS), then
change the kernel names/mac mapping appropriately. Where can a
duplicate name become an issue using this method?
--
dann frazier
^ permalink raw reply
* RE: How to use gretap with bridge?
From: Neulinger, Nathan @ 2009-10-30 17:08 UTC (permalink / raw)
To: Herbert Xu, David S. Miller; +Cc: shemminger, netdev
In-Reply-To: <20091030155148.GA8283@gondor.apana.org.au>
Confirmed, this fixes the problem for me and even now allows changing
the mac access by request.
Now I'm on to figuring out MTU issues. Thanks!
-- Nathan
------------------------------------------------------------
Nathan Neulinger nneul@mst.edu
Missouri S&T Information Technology (573) 612-1412
System Administrator - Principal KD0DMH
> -----Original Message-----
> From: Herbert Xu [mailto:herbert@gondor.apana.org.au]
> Sent: Friday, October 30, 2009 10:52 AM
> To: Neulinger, Nathan; David S. Miller
> Cc: shemminger@vyatta.com; netdev@vger.kernel.org
> Subject: Re: How to use gretap with bridge?
>
> Neulinger, Nathan <nneul@mst.edu> wrote:
> >
> > The above change fixes it for me, but I'm no expert on this chunk of
> > code. (Perhaps it it shouldn't set dev_addr at all?)
>
> OK, it was a stupid mistake on my part. I added a netdev ops
> struct for tap but didn't actually use it! Please let us know
> whether this patch fixes the problem.
>
> gre: Fix dev_addr clobbering for gretap
>
> Nathan Neulinger noticed that gretap devices get their MAC address
> from the local IP address, which results in invalid MAC addresses
> half of the time.
>
> This is because gretap is still using the tunnel netdev ops rather
> than the correct tap netdev ops struct.
>
> This patch also fixes changelink to not clobber the MAC address
> for the gretap case.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> Thanks,
> --
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> --
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 41ada99..1433338 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -1464,7 +1464,7 @@ static void ipgre_tap_setup(struct net_device
> *dev)
>
> ether_setup(dev);
>
> - dev->netdev_ops = &ipgre_netdev_ops;
> + dev->netdev_ops = &ipgre_tap_netdev_ops;
> dev->destructor = free_netdev;
>
> dev->iflink = 0;
> @@ -1525,25 +1525,29 @@ static int ipgre_changelink(struct net_device
> *dev, struct nlattr *tb[],
> if (t->dev != dev)
> return -EEXIST;
> } else {
> - unsigned nflags = 0;
> -
> t = nt;
>
> - if (ipv4_is_multicast(p.iph.daddr))
> - nflags = IFF_BROADCAST;
> - else if (p.iph.daddr)
> - nflags = IFF_POINTOPOINT;
> + if (dev->type != ARPHRD_ETHER) {
> + unsigned nflags = 0;
>
> - if ((dev->flags ^ nflags) &
> - (IFF_POINTOPOINT | IFF_BROADCAST))
> - return -EINVAL;
> + if (ipv4_is_multicast(p.iph.daddr))
> + nflags = IFF_BROADCAST;
> + else if (p.iph.daddr)
> + nflags = IFF_POINTOPOINT;
> +
> + if ((dev->flags ^ nflags) &
> + (IFF_POINTOPOINT | IFF_BROADCAST))
> + return -EINVAL;
> + }
>
> ipgre_tunnel_unlink(ign, t);
> t->parms.iph.saddr = p.iph.saddr;
> t->parms.iph.daddr = p.iph.daddr;
> t->parms.i_key = p.i_key;
> - memcpy(dev->dev_addr, &p.iph.saddr, 4);
> - memcpy(dev->broadcast, &p.iph.daddr, 4);
> + if (dev->type != ARPHRD_ETHER) {
> + memcpy(dev->dev_addr, &p.iph.saddr, 4);
> + memcpy(dev->broadcast, &p.iph.daddr, 4);
> + }
> ipgre_tunnel_link(ign, t);
> netdev_state_change(dev);
> }
^ permalink raw reply
* Re: [PATCH] udev: create empty regular files to represent net interfaces
From: Matt Domsch @ 2009-10-30 17:10 UTC (permalink / raw)
To: dann frazier
Cc: Narendra_K, greg, kay.sievers, linux-hotplug, netdev,
Jordan_Hargrave, Charles_Rose, bhutchings
In-Reply-To: <20091030160845.GA4547@lackof.org>
On Fri, Oct 30, 2009 at 10:08:45AM -0600, dann frazier wrote:
> On Fri, Oct 30, 2009 at 08:43:44PM +0530, Narendra_K@Dell.com wrote:
> >
> > >> This way the kernel has only one name, and so does userspace, and
> > >> everyone is happy.
> > >
> > >There are two issues, which really seem distinct to me.
> > >
> > >Users expect eth0 to map to first-onboard-nic. That's an
> > >installer issue (since the BIOS can already export this info)
> > >and I agree that if we want to "fix" that, we should fix it there.
> > >
> >
> > I agree that installers have to be fixed in the sense that they can be
> > told to find the right interface. But, they expect determinism and
> > depend on "eth0 to map to first-onboard-nic". Installer is one of the
> > applications that is affected by this and needs user intervention, if it
> > is not told about the right interface. I discussed installer as it is so
> > much part of a user experience.
>
> Right, but couldn't the installer do the work of scanning the SMBIOS
> to figure out which nics are onboard, and reorder the 'eth*' names
> such that these are first? This state could then be written out as
> udev rules so that they persist across reboots.
No, there is a catch-22. To be sure you know the "proper" ethN name
to assign a device based on an ordering, you have to know about all
the devices. When udev runs, one device at a time, it can only see
the current device and all those that have come before it, but it can't know
when all the drivers for all the NICs have been loaded. And if you
hotplug a device in later, it should presumably just go at the end of
the list, but after a reboot, it'll most likely show up somewhere in
the middle of the list. SMBIOS is static from boottime, not hotplug
aware. If I add a 4-port NIC in slot 3 after boot, it becomes
ethN..N+3. After reboot, it may well show up at completely different
places, and even the N..N+3 ordering of individiual ports on the card
aren't guaranteed to be consistent.
ethN is fundamentally a nondeterministic namespace, and trying to
enforce determinism on it is, from all my attempts, impossible. Hence
the desire to change the namespace. But there can be many
different naming policies one might want (including the
nondeterministic ethN policy), and for all other types of devices this
isn't a problem - we can have all the policies we want, in parallel.
Only for network devices we can't.
Stephen, I hadn't seen the ifalias field you added. I can see that
being helpful to a user (some tool can write a more meaningful string
to it), but I can't see it being useful programmatically. It still
doesn't get me to "ifconfig the NIC in slot 3 port 2" or "ifconfig the
NIC I booted from".
Thanks,
Matt
--
Matt Domsch
Technology Strategist, Dell Office of the CTO
linux.dell.com & www.dell.com/linux
^ permalink raw reply
* [PATCH net-next-2.6] net: Introduce dev_get_by_name_rcu()
From: Eric Dumazet @ 2009-10-30 17:11 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
Some workloads hit dev_base_lock rwlock pretty hard.
We can use RCU lookups to avoid touching this rwlock
(and avoid touching netdevice refcount)
netdevices are already freed after a RCU grace period, so this patch
adds no penalty at device dismantle time.
However, it adds a synchronize_rcu() call in dev_change_name()
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/netdevice.h | 1
net/core/dev.c | 49 +++++++++++++++++++++++++++++-------
2 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 193b637..86773e1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1115,6 +1115,7 @@ extern void __dev_remove_pack(struct packet_type *pt);
extern struct net_device *dev_get_by_flags(struct net *net, unsigned short flags,
unsigned short mask);
extern struct net_device *dev_get_by_name(struct net *net, const char *name);
+extern struct net_device *dev_get_by_name_rcu(struct net *net, const char *name);
extern struct net_device *__dev_get_by_name(struct net *net, const char *name);
extern int dev_alloc_name(struct net_device *dev, const char *name);
extern int dev_open(struct net_device *dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index 94f42a1..f54d8b8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -213,7 +213,7 @@ static int list_netdevice(struct net_device *dev)
write_lock_bh(&dev_base_lock);
list_add_tail(&dev->dev_list, &net->dev_base_head);
- hlist_add_head(&dev->name_hlist, dev_name_hash(net, dev->name));
+ hlist_add_head_rcu(&dev->name_hlist, dev_name_hash(net, dev->name));
hlist_add_head_rcu(&dev->index_hlist,
dev_index_hash(net, dev->ifindex));
write_unlock_bh(&dev_base_lock);
@@ -230,7 +230,7 @@ static void unlist_netdevice(struct net_device *dev)
/* Unlink dev from the device chain */
write_lock_bh(&dev_base_lock);
list_del(&dev->dev_list);
- hlist_del(&dev->name_hlist);
+ hlist_del_rcu(&dev->name_hlist);
hlist_del_rcu(&dev->index_hlist);
write_unlock_bh(&dev_base_lock);
}
@@ -599,6 +599,32 @@ struct net_device *__dev_get_by_name(struct net *net, const char *name)
EXPORT_SYMBOL(__dev_get_by_name);
/**
+ * dev_get_by_name_rcu - find a device by its name
+ * @net: the applicable net namespace
+ * @name: name to find
+ *
+ * Find an interface by name.
+ * If the name is found a pointer to the device is returned.
+ * If the name is not found then %NULL is returned.
+ * The reference counters are not incremented so the caller must be
+ * careful with locks. The caller must hold RCU lock.
+ */
+
+struct net_device *dev_get_by_name_rcu(struct net *net, const char *name)
+{
+ struct hlist_node *p;
+ struct net_device *dev;
+ struct hlist_head *head = dev_name_hash(net, name);
+
+ hlist_for_each_entry_rcu(dev, p, head, name_hlist)
+ if (!strncmp(dev->name, name, IFNAMSIZ))
+ return dev;
+
+ return NULL;
+}
+EXPORT_SYMBOL(dev_get_by_name_rcu);
+
+/**
* dev_get_by_name - find a device by its name
* @net: the applicable net namespace
* @name: name to find
@@ -614,11 +640,11 @@ struct net_device *dev_get_by_name(struct net *net, const char *name)
{
struct net_device *dev;
- read_lock(&dev_base_lock);
- dev = __dev_get_by_name(net, name);
+ rcu_read_lock();
+ dev = dev_get_by_name_rcu(net, name);
if (dev)
dev_hold(dev);
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return dev;
}
EXPORT_SYMBOL(dev_get_by_name);
@@ -960,7 +986,12 @@ rollback:
write_lock_bh(&dev_base_lock);
hlist_del(&dev->name_hlist);
- hlist_add_head(&dev->name_hlist, dev_name_hash(net, dev->name));
+ write_unlock_bh(&dev_base_lock);
+
+ synchronize_rcu();
+
+ write_lock_bh(&dev_base_lock);
+ hlist_add_head_rcu(&dev->name_hlist, dev_name_hash(net, dev->name));
write_unlock_bh(&dev_base_lock);
ret = call_netdevice_notifiers(NETDEV_CHANGENAME, dev);
@@ -1062,9 +1093,9 @@ void dev_load(struct net *net, const char *name)
{
struct net_device *dev;
- read_lock(&dev_base_lock);
- dev = __dev_get_by_name(net, name);
- read_unlock(&dev_base_lock);
+ rcu_read_lock();
+ dev = dev_get_by_name_rcu(net, name);
+ rcu_read_unlock();
if (!dev && capable(CAP_NET_ADMIN))
request_module("%s", name);
^ permalink raw reply related
* Re: [PATCH] udev: create empty regular files to represent net interfaces
From: Greg KH @ 2009-10-30 17:13 UTC (permalink / raw)
To: Matt Domsch
Cc: dann frazier, Narendra_K, kay.sievers, linux-hotplug, netdev,
Jordan_Hargrave, Charles_Rose, bhutchings
In-Reply-To: <20091030171003.GA7523@auslistsprd01.us.dell.com>
On Fri, Oct 30, 2009 at 12:10:03PM -0500, Matt Domsch wrote:
> ethN is fundamentally a nondeterministic namespace, and trying to
> enforce determinism on it is, from all my attempts, impossible. Hence
> the desire to change the namespace. But there can be many
> different naming policies one might want (including the
> nondeterministic ethN policy), and for all other types of devices this
> isn't a problem - we can have all the policies we want, in parallel.
> Only for network devices we can't.
So pick one in your installer and stick with it. Doesn't seem that
complicated to me...
greg k-h
^ permalink raw reply
* Re: [PATCH 2/3] net: TCP thin linear timeouts
From: Rick Jones @ 2009-10-30 17:33 UTC (permalink / raw)
To: apetlund
Cc: Ilpo Järvinen, Arnd Hannemann, Eric Dumazet, Netdev, LKML,
shemminger, David Miller
In-Reply-To: <6980f83370cc081eb82dc2e0bd65bcf4.squirrel@webmail.uio.no>
apetlund@simula.no wrote:
>> Just how thin can a thin stream be when a thin stream is found thin? (to
>> the cadence of "How much wood could a woodchuck chuck if a woodchuck could
>> chuck wood?")
>> Does a stream get so thin that a user's send could not be split into four,
>> sub-MSS TCP segments?
>
>
> That was a nifty idea: Anti-Nagle the segments to be able to trigger fast
> retransmissions. I think it is possible.
>
> Besides using more resources on each send, this scheme will introduce the
> need to delay parts of the segment, which is undesirable for
> time-dependent applications (the intended target of the mechanisms).
>
> I think it would be fun to implement and play around with such a mechanism
> to see the effects.
Indeed, it does feel a bit "anti-nagle" but at the same time, these thin streams
are supposed to be quite rare right? I mean we have survived 20 odd years of
congestion control and fast retransmission without it being a big issue.
They are also supposed to not have terribly high bandwidth requirements yes?
Suppose that instead of an explicit "I promise to be thin" setsockopt(), they
instead set a Very Small (tm) in today's thinking socket buffer size and the
stack then picks the MSS to be no more than 1/4 that size? Or for that matter,
assuming the permissions are acceptable, the thin application makes a
setsockopt(TCP_MAXSEG) call such that the actual MSS is small enough to allow
the send()'s to be four (or more) segments. And, if one wants to spin-away the
anti-Nagle, Nagle is defined by the send() being smaller than the MSS, so if the
MSS is smaller, it isn't anti-Nagle :)
Further blue-skying...
If SACK were also enabled, it would seem that only loss of the last segment in
the "thin train" would be an issue? Presumably, the thin stream receiver would
be in a position to detect this, perhaps with an application-level timeout.
Whether then it would suffice to allow the receiving app to make a setsockopt()
call to force an extra ACK or two I'm not sure. Perhaps if the thin-stream had
a semi-aggressive "heartbeat" going...
But it does seem that it should be possible to deal with this sort of thing
without having to make wholesale changes to TCP's RTO policies and whatnot?
rick jones
^ permalink raw reply
* RFC: netdev: allow ethtool physical id to drop rtnl_lock
From: Stephen Hemminger @ 2009-10-30 17:42 UTC (permalink / raw)
To: David Miller; +Cc: netdev
The ethtool operation to blink LED can take an indeterminately long time,
blocking out other operations (via rtnl_lock). This patch is an attempt
to work around the problem.
It does need more discussion, because it will mean that drivers that formerly
were protected from changes during blink aren't. For example, user could
start device blinking, and then plug in cable causing change netlink event
to change state or pull cable and have device come down.
The other possibility is to do this on a driver by driver basis
which is more effort.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/core/ethtool.c 2009-10-30 10:27:23.621917624 -0700
+++ b/net/core/ethtool.c 2009-10-30 10:35:53.787670774 -0700
@@ -17,6 +17,7 @@
#include <linux/errno.h>
#include <linux/ethtool.h>
#include <linux/netdevice.h>
+#include <linux/rtnetlink.h>
#include <asm/uaccess.h>
/*
@@ -781,6 +782,8 @@ static int ethtool_get_strings(struct ne
static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
{
struct ethtool_value id;
+ int err;
+ static int busy;
if (!dev->ethtool_ops->phys_id)
return -EOPNOTSUPP;
@@ -788,7 +791,21 @@ static int ethtool_phys_id(struct net_de
if (copy_from_user(&id, useraddr, sizeof(id)))
return -EFAULT;
- return dev->ethtool_ops->phys_id(dev, id.data);
+ if (busy)
+ return -EBUSY;
+
+ /* This operation may take a long time, drop lock */
+ busy = 1;
+ dev_hold(dev);
+ rtnl_unlock();
+
+ err = dev->ethtool_ops->phys_id(dev, id.data);
+
+ rtnl_lock();
+ dev_put(dev);
+ busy = 0;
+
+ return err;
}
static int ethtool_get_stats(struct net_device *dev, void __user *useraddr)
^ permalink raw reply
* Re: RFC: netdev: allow ethtool physical id to drop rtnl_lock
From: Eric Dumazet @ 2009-10-30 17:47 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20091030104201.6bb03b23@nehalam>
Stephen Hemminger a écrit :
> The ethtool operation to blink LED can take an indeterminately long time,
> blocking out other operations (via rtnl_lock). This patch is an attempt
> to work around the problem.
>
> It does need more discussion, because it will mean that drivers that formerly
> were protected from changes during blink aren't. For example, user could
> start device blinking, and then plug in cable causing change netlink event
> to change state or pull cable and have device come down.
>
> The other possibility is to do this on a driver by driver basis
> which is more effort.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
>
> --- a/net/core/ethtool.c 2009-10-30 10:27:23.621917624 -0700
> +++ b/net/core/ethtool.c 2009-10-30 10:35:53.787670774 -0700
> @@ -17,6 +17,7 @@
> #include <linux/errno.h>
> #include <linux/ethtool.h>
> #include <linux/netdevice.h>
> +#include <linux/rtnetlink.h>
> #include <asm/uaccess.h>
>
> /*
> @@ -781,6 +782,8 @@ static int ethtool_get_strings(struct ne
> static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
> {
> struct ethtool_value id;
> + int err;
> + static int busy;
>
> if (!dev->ethtool_ops->phys_id)
> return -EOPNOTSUPP;
> @@ -788,7 +791,21 @@ static int ethtool_phys_id(struct net_de
> if (copy_from_user(&id, useraddr, sizeof(id)))
> return -EFAULT;
>
> - return dev->ethtool_ops->phys_id(dev, id.data);
> + if (busy)
> + return -EBUSY;
> +
> + /* This operation may take a long time, drop lock */
> + busy = 1;
> + dev_hold(dev);
> + rtnl_unlock();
> +
> + err = dev->ethtool_ops->phys_id(dev, id.data);
> +
> + rtnl_lock();
> + dev_put(dev);
> + busy = 0;
> +
> + return err;
> }
>
It seems reasonable, but why have a global 'busy' flag, and not
private to each netdev ?
^ permalink raw reply
* Re: [PATCH] sky2: set carrier off in probe
From: David Miller @ 2009-10-30 18:10 UTC (permalink / raw)
To: herbert; +Cc: bphilips, shemminger, netdev
In-Reply-To: <20091030155420.GA8345@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 30 Oct 2009 11:54:20 -0400
> David Miller <davem@davemloft.net> wrote:
>>
>> I agree that we should report something consistent before interface
>> up and 'no' is probably the best.
>>
>> I remember fixing something similar in other drivers a few months
>> ago.
>
> Can't we do this in one spot rather than having every driver
> duplicate this?
It doesn't matter if we do.
Because the driver must start in state with carrier off anyways,
so that we get the transition event when the device comes up
from link down to link up.
So many things depend upon that link state transition event, that
we're not saving anything by just mucking with the carrier test.
^ permalink raw reply
* Re: [PATCH] sky2: set carrier off in probe
From: David Miller @ 2009-10-30 18:11 UTC (permalink / raw)
To: shemminger; +Cc: bphilips, netdev
In-Reply-To: <20091030083452.68ca227e@nehalam>
From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Fri, 30 Oct 2009 08:34:52 -0700
> Why not fix the problem in a generic way?
Drivers still need to make sure carrier is off when their
->open() routine runs, so that the transition event from
link down to link up occurs properly when the device is brought
up.
So mucking around with this carrier test will only hide the
bugs, not make things easier.
^ permalink raw reply
* Re: [PATCH 2/3] net: TCP thin linear timeouts
From: William Allen Simpson @ 2009-10-30 18:11 UTC (permalink / raw)
To: Rick Jones
Cc: apetlund, Ilpo Järvinen, Arnd Hannemann, Eric Dumazet,
Netdev, LKML, shemminger, David Miller
In-Reply-To: <4AEB2362.5060601@hp.com>
Rick Jones wrote:
> apetlund@simula.no wrote:
>>> Just how thin can a thin stream be when a thin stream is found thin?
>>> (to the cadence of "How much wood could a woodchuck chuck if a
>>> woodchuck could chuck wood?")
>
>>> Does a stream get so thin that a user's send could not be split into
>>> four,
>>> sub-MSS TCP segments?
>>
>>
>> That was a nifty idea: Anti-Nagle the segments to be able to trigger fast
>> retransmissions. I think it is possible.
>>
>> Besides using more resources on each send, this scheme will introduce the
>> need to delay parts of the segment, which is undesirable for
>> time-dependent applications (the intended target of the mechanisms).
>>
>> I think it would be fun to implement and play around with such a
>> mechanism
>> to see the effects.
>
> Indeed, it does feel a bit "anti-nagle" but at the same time, these thin
> streams are supposed to be quite rare right? I mean we have survived 20
> odd years of congestion control and fast retransmission without it being
> a big issue.
>
> They are also supposed to not have terribly high bandwidth requirements
> yes? Suppose that instead of an explicit "I promise to be thin"
> setsockopt(), they instead set a Very Small (tm) in today's thinking
> socket buffer size and the stack then picks the MSS to be no more than
> 1/4 that size? Or for that matter, assuming the permissions are
> acceptable, the thin application makes a setsockopt(TCP_MAXSEG) call
> such that the actual MSS is small enough to allow the send()'s to be
> four (or more) segments. And, if one wants to spin-away the anti-Nagle,
> Nagle is defined by the send() being smaller than the MSS, so if the MSS
> is smaller, it isn't anti-Nagle :)
>
This is not a new idea. Folks used to set the MSS really low for M$
Windows, so that their short little packets went over dialup links more
quickly and they saw a little bit more of their graphic as it crawled to
the screen. Even though it was actually slower in total time, it "felt"
faster because of the continuing visual feedback. It depended upon VJ
Header Prediction to keep the overhead down for the link.
These are/were called "TCP mice", and the result was routers and servers
being nibbled by mice. Not pleasant.
> Further blue-skying...
>
> If SACK were also enabled, it would seem that only loss of the last
> segment in the "thin train" would be an issue? Presumably, the thin
> stream receiver would be in a position to detect this, perhaps with an
> application-level timeout. Whether then it would suffice to allow the
> receiving app to make a setsockopt() call to force an extra ACK or two
> I'm not sure. Perhaps if the thin-stream had a semi-aggressive
> "heartbeat" going...
>
Heartbeats are the usual solution for gaming. Handles a host of
issues, including detection of clients that have become unreachable.
(No, these are not the same as TCP keep-alives.)
Beside my code in the field and widespread discussion, I know that Paul
Francis had several related papers a decade or so ago. My memory is that
younger game coders weren't particularly avid readers....
> But it does seem that it should be possible to deal with this sort of
> thing without having to make wholesale changes to TCP's RTO policies and
> whatnot?
>
Yep.
^ permalink raw reply
* Re: [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie
From: William Allen Simpson @ 2009-10-30 18:11 UTC (permalink / raw)
To: Linux Kernel Network Developers
In-Reply-To: <4AEAC763.4070200@gmail.com>
William Allen Simpson wrote:
> First of all, this is my first attempt at locks, so I'd like early review.
>
> Secondly, scripts/checkpatch.pl tells me:
>
> ERROR: do not initialise statics to 0 or NULL
> #95: FILE: net/ipv4/tcp.c:2977:
> +static struct tcp_cookie_secret *tcp_secret_generating = NULL;
>
> They need to be NULL, and I'm not planning on exporting them, so what's
> the preferred mechanism?
>
> (I've grep'd many other instances of statics = 0 or NULL, so I'm not
> alone.)
>
I've found a void __init tcp_init() that seems to be used to initialize
some things, so I've stuck the NULL in there.
Still would like somebody to assure me that I've used the locks correctly.
^ permalink raw reply
* Re: [PATCH] sky2: set carrier off in probe
From: Herbert Xu @ 2009-10-30 18:16 UTC (permalink / raw)
To: David Miller; +Cc: bphilips, shemminger, netdev
In-Reply-To: <20091030.111027.234994529.davem@davemloft.net>
On Fri, Oct 30, 2009 at 11:10:27AM -0700, David Miller wrote:
.
> Because the driver must start in state with carrier off anyways,
> so that we get the transition event when the device comes up
> from link down to link up.
Well we want all drivers to start in state NOCARRIER. However,
a freshly allocated netdev has the NOCARRIER bit off. This means
every single driver has to set the NOCARRIER bit.
It would seem much easier to ensure that the NOCARRIER bit is set
in a newly allocated netdev.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox