* [PATCH] tcp resets are misrouted
From: Alexey Kuznetsov @ 2012-10-12 14:34 UTC (permalink / raw)
To: netdev, davem, shawn.lu, eric.dumazet, sol
After commit e2446eaa.. tcp resets are always lost, when routing is asymmetric.
Yes, backing out that patch will result in misrouting of resets for dead connections
which used interface binding when were alive, but we actually cannot do anything here.
What's died that's died and correct handling normal unbound connections is obviously a priority.
Comment to comment:
> This has few benefits:
> 1. tcp_v6_send_reset already did that.
It was done to route resets for IPv6 link local addresses. It was a mistake to
do so for global addresses. The patch fixes this as well.
Actually, the problem appears to be even more serious than guaranteed loss of resets.
As reported by Sergey Soloviev <sol@eqv.ru>, those misrouted resets create a lot of
arp traffic and huge amount of unresolved arp entires putting down to knees NAT firewalls
which use asymmetric routing.
Signed-off-by: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
---
net/ipv4/tcp_ipv4.c | 7 ++++---
net/ipv6/tcp_ipv6.c | 3 ++-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 75735c9..ef998b0 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -708,10 +708,11 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
arg.csumoffset = offsetof(struct tcphdr, check) / 2;
arg.flags = (sk && inet_sk(sk)->transparent) ? IP_REPLY_ARG_NOSRCCHECK : 0;
/* When socket is gone, all binding information is lost.
- * routing might fail in this case. using iif for oif to
- * make sure we can deliver it
+ * routing might fail in this case. No choice here, if we choose to force
+ * input interface, we will misroute in case of asymmetric route.
*/
- arg.bound_dev_if = sk ? sk->sk_bound_dev_if : inet_iif(skb);
+ if (sk)
+ arg.bound_dev_if = sk->sk_bound_dev_if;
net = dev_net(skb_dst(skb)->dev);
arg.tos = ip_hdr(skb)->tos;
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 49c8903..26175bf 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -877,7 +877,8 @@ static void tcp_v6_send_response(struct sk_buff *skb, u32 seq, u32 ack, u32 win,
__tcp_v6_send_check(buff, &fl6.saddr, &fl6.daddr);
fl6.flowi6_proto = IPPROTO_TCP;
- fl6.flowi6_oif = inet6_iif(skb);
+ if (ipv6_addr_type(&fl6.daddr) & IPV6_ADDR_LINKLOCAL)
+ fl6.flowi6_oif = inet6_iif(skb);
fl6.fl6_dport = t1->dest;
fl6.fl6_sport = t1->source;
security_skb_classify_flow(skb, flowi6_to_flowi(&fl6));
--
1.7.2.3
^ permalink raw reply related
* Re: alignment faults in 3.6
From: Ben Hutchings @ 2012-10-12 15:04 UTC (permalink / raw)
To: Eric Dumazet
Cc: Benjamin LaHaise, Russell King - ARM Linux,
Måns Rullgård, Arnd Bergmann, linux-arm-kernel,
David Laight, netdev, Jon Masters
In-Reply-To: <1350053300.21172.12319.camel@edumazet-glaptop>
On Fri, 2012-10-12 at 16:48 +0200, Eric Dumazet wrote:
> On Fri, 2012-10-12 at 10:22 -0400, Benjamin LaHaise wrote:
> > On Fri, Oct 12, 2012 at 02:08:12PM +0200, Eric Dumazet wrote:
> > > So yes, we built network stack with the prereq that IP headers are
> > > aligned, but unfortunately many developers use x86 which doesnt care, so
> > > its possible some bugs are added.
> >
> > x86 does have an alignment check flag that can be set in the flags register.
> > Somehow, I doubt anyone would be willing to walk through all the noise the
> > faults would likely trigger.
>
> If this can be mapped to an event that can be used by perf tool, that
> might be useful ?
AC was so useless that it has now been reallocated to use by SMAP
<https://lwn.net/Articles/517251/>.
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: alignment faults in 3.6
From: Benjamin LaHaise @ 2012-10-12 15:00 UTC (permalink / raw)
To: Eric Dumazet
Cc: Russell King - ARM Linux, Måns Rullgård, Arnd Bergmann,
linux-arm-kernel, David Laight, netdev, Jon Masters
In-Reply-To: <1350053300.21172.12319.camel@edumazet-glaptop>
On Fri, Oct 12, 2012 at 04:48:20PM +0200, Eric Dumazet wrote:
> > Somehow, I doubt anyone would be willing to walk through all the noise the
> > faults would likely trigger.
>
> If this can be mapped to an event that can be used by perf tool, that
> might be useful ?
There are performance counters for the various different types of alignment
faults supported by perf. Modern x86 makes the vast majority of unaligned
accesses very low overhead -- the only ones that really hurt are those
straddling different vm pages, but even those have little cost compared to
obsolete microarchitectures (*cough* P4 *cough*).
-ben
--
"Thought is the essence of where you are now."
^ permalink raw reply
* Re: [PATCH v2 1/4] net: mvneta: driver for Marvell Armada 370/XP network unit
From: Thomas Petazzoni @ 2012-10-12 14:59 UTC (permalink / raw)
To: Jason Cooper
Cc: Francois Romieu, David S. Miller, Lennert Buytenhek, netdev,
linux-arm-kernel, Andrew Lunn, Gregory Clement, Lior Amsalem,
Maen Suleiman, Rami Rosen
In-Reply-To: <20121012143131.GP12330@titan.lakedaemon.net>
Jason,
On Fri, 12 Oct 2012 10:31:31 -0400, Jason Cooper wrote:
> I agree with Francois on most of these. I prefer readability over
> hard 80 column limits.
Sure, but checkpatch.pl is warning on every line exceeding the 80
columns. Not that I think that all checkpatch.pl warnings should
necessarily be religiously respected, but if you have gazillions of
warnings regarding line exceeding 80 columns, it is very likely that
you will miss more important warnings.
> Although, 80 columns is still sound
> guidance. For example, a majority of the broken lines are due to
> long macro and constant names. I did a 'git grep NETA' and didn't
> see anything alarming. So, above could become
>
> val |= rx_filled << NETA_RXQ_ADD_NONOCC_SHIFT;
I don't mind, but then I would like to keep things consistent:
* The driver file would be neta.c
* All functions and data structure would be prefixed neta_ and not
mvneta_
* The Kconfig option would become CONFIG_NETA. Do we really want such
a "simple" Kconfig option name for a driver?
Also, this is entirely not consistent with the existing mv643xx_eth
driver, which has kept the "mv" part of the prefix.
Maybe the fact that those long macros are making long lines is also due
to the code having sometimes a too deep indentation, and I need to fix
that by using more auxiliary functions or something like that?
Thanks,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH net-next] unix: Remove unused field from unix_sock
From: Pavel Emelyanov @ 2012-10-12 14:53 UTC (permalink / raw)
To: David Miller, Linux Netdev List
The struct sock *other one seem to be unused. Grep and make do not object.
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
---
diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index b5f8988..0a996a3 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -53,7 +53,6 @@ struct unix_sock {
struct path path;
struct mutex readlock;
struct sock *peer;
- struct sock *other;
struct list_head link;
atomic_long_t inflight;
spinlock_t lock;
^ permalink raw reply related
* Re: alignment faults in 3.6
From: Eric Dumazet @ 2012-10-12 14:48 UTC (permalink / raw)
To: Benjamin LaHaise
Cc: Russell King - ARM Linux, Måns Rullgård, Arnd Bergmann,
linux-arm-kernel, David Laight, netdev, Jon Masters
In-Reply-To: <20121012142254.GG5453@kvack.org>
On Fri, 2012-10-12 at 10:22 -0400, Benjamin LaHaise wrote:
> On Fri, Oct 12, 2012 at 02:08:12PM +0200, Eric Dumazet wrote:
> > So yes, we built network stack with the prereq that IP headers are
> > aligned, but unfortunately many developers use x86 which doesnt care, so
> > its possible some bugs are added.
>
> x86 does have an alignment check flag that can be set in the flags register.
> Somehow, I doubt anyone would be willing to walk through all the noise the
> faults would likely trigger.
If this can be mapped to an event that can be used by perf tool, that
might be useful ?
^ permalink raw reply
* RE: alignment faults in 3.6
From: David Laight @ 2012-10-12 14:36 UTC (permalink / raw)
To: Benjamin LaHaise, Eric Dumazet
Cc: Russell King - ARM Linux, Måns Rullgård, Arnd Bergmann,
linux-arm-kernel, netdev, Jon Masters
In-Reply-To: <20121012142254.GG5453@kvack.org>
> x86 does have an alignment check flag that can be set in the flags register.
> Somehow, I doubt anyone would be willing to walk through all the noise the
> faults would likely trigger.
Someone has tried to set that (in userspace) on NetBSD.
The fault reporting has been fixed, but really nothing
works since optimised parts of libc deliberately do
misaligned transfers.
David
^ permalink raw reply
* Re: [PATCH v2 1/4] net: mvneta: driver for Marvell Armada 370/XP network unit
From: Jason Cooper @ 2012-10-12 14:31 UTC (permalink / raw)
To: Francois Romieu
Cc: Thomas Petazzoni, David S. Miller, Lennert Buytenhek, netdev,
linux-arm-kernel, Andrew Lunn, Gregory Clement, Lior Amsalem,
Maen Suleiman, Rami Rosen
In-Reply-To: <20121011212629.GA14171@electric-eye.fr.zoreil.com>
On Thu, Oct 11, 2012 at 11:26:29PM +0200, Francois Romieu wrote:
> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> :
> [...]
> > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > new file mode 100644
> > index 0000000..4f7fe08
> > --- /dev/null
> > +++ b/drivers/net/ethernet/marvell/mvneta.c
> [...]
> > +static void mvneta_rxq_desc_num_update(struct mvneta_port *pp,
> > + struct mvneta_rx_queue *rxq,
> > + int rx_done, int rx_filled)
> > +{
> > + u32 val;
> > +
> > + if ((rx_done <= 0xff) && (rx_filled <= 0xff)) {
> > + val = rx_done |
> > + (rx_filled << MVNETA_RXQ_ADD_NON_OCCUPIED_SHIFT);
> > + mvreg_write(pp, MVNETA_RXQ_STATUS_UPDATE_REG(rxq->id), val);
> > + return;
> > + }
> > +
> > + /* Only 255 descriptors can be added at once */
> > + while ((rx_done > 0) || (rx_filled > 0)) {
> > + if (rx_done <= 0xff) {
> > + val = rx_done;
> > + rx_done = 0;
> > + } else {
> > + val = 0xff;
> > + rx_done -= 0xff;
> > + }
> > + if (rx_filled <= 0xff) {
> > + val |= rx_filled
> > + << MVNETA_RXQ_ADD_NON_OCCUPIED_SHIFT;
>
> val |= rx_filled << MVNETA_RXQ_ADD_NON_OCCUPIED_SHIFT;
I agree with Francois on most of these. I prefer readability over hard
80 column limits. Although, 80 columns is still sound guidance. For
example, a majority of the broken lines are due to long macro and
constant names. I did a 'git grep NETA' and didn't see anything
alarming. So, above could become
val |= rx_filled << NETA_RXQ_ADD_NONOCC_SHIFT;
thx,
Jason.
^ permalink raw reply
* performance regression with skb_add_data_nocache
From: Animesh K Trivedi1 @ 2012-10-12 14:29 UTC (permalink / raw)
To: netdev; +Cc: Bernard Metzler, Animesh K Trivedi1
Hi all,
I recently upgraded from 2.6.36 to 3.2.28 and saw performance regression
for
TCP performance. Upon further investigation it looked that
skb_add_data_nocache()
was the culprit.
I am getting following performance numbers on my Nehalem (Xeon E7520) box
connected using 10GbE cards (transmit side, and netperf client). Server is
a another
box with E5540 CPU (receiver of the request) . For my netperf TCP_RR tests:
- 1,400 bytes request, 1 byte response:
No cache copy (enabled) : 26,623 tps, 22.72% utilization
No cache copy (disabled) : 26,710 tps, 21.76% utilization
- 14,000 bytes request, 1 byte response:
No cache copy (enabled) : 14,245 tps, 23.04% utilization
No cache copy (disabled) : 14,850 tps, 21.6% utilization
and for even larger buffer the performance lag is increases with
significant CPU load
- 1 MBytes request, 1 byte response:
No cache copy (enabled) : 1,032 tps, 98.96% utilization
No cache copy (disabled) : 1,081 tps, 74.86% utilization
Though there isn't a lot performance difference, but notice the significant
CPU
utilization in case of nocache copy for 1MB buffer size. Thoughts?
Thanks,
--
Animesh
^ permalink raw reply
* Re: alignment faults in 3.6
From: Benjamin LaHaise @ 2012-10-12 14:22 UTC (permalink / raw)
To: Eric Dumazet
Cc: Russell King - ARM Linux, Måns Rullgård, Arnd Bergmann,
linux-arm-kernel, David Laight, netdev, Jon Masters
In-Reply-To: <1350043692.21172.11815.camel@edumazet-glaptop>
On Fri, Oct 12, 2012 at 02:08:12PM +0200, Eric Dumazet wrote:
> So yes, we built network stack with the prereq that IP headers are
> aligned, but unfortunately many developers use x86 which doesnt care, so
> its possible some bugs are added.
x86 does have an alignment check flag that can be set in the flags register.
Somehow, I doubt anyone would be willing to walk through all the noise the
faults would likely trigger.
-ben
--
"Thought is the essence of where you are now."
^ permalink raw reply
* Re: [PATCH v2 1/4] net: mvneta: driver for Marvell Armada 370/XP network unit
From: Thomas Petazzoni @ 2012-10-12 14:16 UTC (permalink / raw)
To: Rob Herring
Cc: David S. Miller, Lennert Buytenhek, Lior Amsalem, Andrew Lunn,
Jason Cooper, Rami Rosen, netdev, Maen Suleiman, Gregory Clement,
linux-arm-kernel
In-Reply-To: <5076E9F1.60200@gmail.com>
Rob,
On Thu, 11 Oct 2012 10:46:57 -0500, Rob Herring wrote:
> > +Required properties:
> > +- compatible: should be "marvell,neta".
>
> This should be more specific such as "marvell,armada-xp-neta".
>
> Or use 370 instead of xp. It should be which ever chip came first.
Is this really useful? The name of this network unit in Marvell is
simply "neta", and since it is associated with the vendor name Marvell
in the compatible string, it is actually unique: "marvell,neta".
The thing is that this unit is used in Armada 370, Armada XP, but also
other SoCs (which I am not sure are announced publicly as of today). So
if possible, we would prefer to keep the proposed "marvell,neta" name.
We could also change it to "marvell,ebu-neta" where EBU stands for
Embedded Business Unit, just like the "ebu" in arch/arm/mach-mvebu/,
but it sounds more logical to have just "marvell,neta".
Thanks again for your review,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 01/12] usbnet: introduce usbnet 3 command helpers
From: Oliver Neukum @ 2012-10-12 13:51 UTC (permalink / raw)
To: Alan Stern
Cc: Ming Lei, David S. Miller, Greg Kroah-Hartman,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
jkosina-IBi9RG/b67k
In-Reply-To: <Pine.LNX.4.44L0.1210111030570.1170-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
On Thursday 11 October 2012 10:36:22 Alan Stern wrote:
> It's worse than you may realize. When a SCSI disk is suspended, all of
> its ancestor devices may be suspended too. Pages can't be read in from
> the drive until all those ancestors are resumed. This means that all
> runtime resume code paths for all drivers that could be bound to an
> ancestor of a block device must avoid GFP_KERNEL. In practice it's
> probably easiest for the runtime PM core to use tsk_set_allowd_gfp()
> before calling any runtime_resume method.
>
> Or at least, this will be true when sd supports nontrivial autosuspend.
Up to now, I've found three driver for which tsk_set_allowd_gfp() wouldn't
do the job. They boil down into two types of errors. That is surprisingly good.
First we have workqueues. bas-gigaset is a good example.
The driver kills a scheduled work in pre_reset(). If this is done synchronously
the driver may need to wait for a memory allocation inside the work.
In principle we could provide a workqueue limited to GFP_NOIO. Is that worth
it, or do we just check?
Second there is a problem just like priority inversion with realtime tasks.
usb-skeleton and ati_remote2
They take mutexes which are also taken in other code paths. So the error
handler may need to wait for a mutex to be dropped which can only happen
if a memory allocation succeeds, which is waiting for the error handler.
usb-skeleton is even worse, as it does copy_to_user(). I guess copy_to/from_user
must simply not be done under such a mutex.
I am afraid there is no generic solution in the last two cases. What do you think?
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] phy: add AT803x driver
From: ujhelyi.m @ 2012-10-12 13:47 UTC (permalink / raw)
To: netdev; +Cc: davem, daniel, rd, Matus Ujhelyi
From: Matus Ujhelyi <ujhelyi.m@gmail.com>
This driver add support for wake over lan on AT803x phys.
Signed-off-by: Matus Ujhelyi <ujhelyi.m@gmail.com>
---
drivers/net/phy/Kconfig | 5 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/at803x.c | 176 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 182 insertions(+)
create mode 100644 drivers/net/phy/at803x.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 3090dc6..e69d74e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -15,6 +15,11 @@ if PHYLIB
comment "MII PHY device drivers"
+config AT803X_PHY
+ tristate "Drivers for Atheros PHYs"
+ ---help---
+ Currently supports the AT8030 and AT8035 model
+
config AMD_PHY
tristate "Drivers for the AMD PHYs"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 6d2dc6c..66720a5 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_STE10XP) += ste10Xp.o
obj-$(CONFIG_MICREL_PHY) += micrel.o
obj-$(CONFIG_MDIO_OCTEON) += mdio-octeon.o
obj-$(CONFIG_MICREL_KS8995MA) += spi_ks8995.o
+obj-$(CONFIG_AT803X_PHY) += at803x.o
obj-$(CONFIG_AMD_PHY) += amd.o
obj-$(CONFIG_MDIO_BUS_MUX) += mdio-mux.o
obj-$(CONFIG_MDIO_BUS_MUX_GPIO) += mdio-mux-gpio.o
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
new file mode 100644
index 0000000..45cbc10
--- /dev/null
+++ b/drivers/net/phy/at803x.c
@@ -0,0 +1,176 @@
+/*
+ * drivers/net/phy/at803x.c
+ *
+ * Driver for Atheros 803x PHY
+ *
+ * Author: Matus Ujhelyi <ujhelyi.m@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/phy.h>
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+
+#define AT803X_INTR_ENABLE 0x12
+#define AT803X_INTR_STATUS 0x13
+#define AT803X_WOL_ENABLE 0x01
+#define AT803X_DEVICE_ADDR 0x03
+#define AT803X_LOC_MAC_ADDR_0_15_OFFSET 0x804C
+#define AT803X_LOC_MAC_ADDR_16_31_OFFSET 0x804B
+#define AT803X_LOC_MAC_ADDR_32_47_OFFSET 0x804A
+#define AT803X_MMD_ACCESS_CONTROL 0x0D
+#define AT803X_MMD_ACCESS_CONTROL_DATA 0x0E
+#define AT803X_FUNC_DATA 0x4003
+
+MODULE_DESCRIPTION("Atheros 803x PHY driver");
+MODULE_AUTHOR("Matus Ujhelyi");
+MODULE_LICENSE("GPL");
+
+static void at803x_set_wol_mac_addr(struct phy_device *phydev)
+{
+ struct net_device *ndev = phydev->attached_dev;
+ const u8 *mac;
+ unsigned int i, offsets[] = {
+ AT803X_LOC_MAC_ADDR_32_47_OFFSET,
+ AT803X_LOC_MAC_ADDR_16_31_OFFSET,
+ AT803X_LOC_MAC_ADDR_0_15_OFFSET,
+ };
+
+ if (!ndev)
+ return;
+
+ mac = (const u8 *) ndev->dev_addr;
+
+ if (!is_valid_ether_addr(mac))
+ return;
+
+ for (i = 0; i < 3; i++) {
+ phy_write(phydev, AT803X_MMD_ACCESS_CONTROL,
+ AT803X_DEVICE_ADDR);
+ phy_write(phydev, AT803X_MMD_ACCESS_CONTROL_DATA,
+ offsets[i]);
+ phy_write(phydev, AT803X_MMD_ACCESS_CONTROL,
+ AT803X_FUNC_DATA);
+ phy_write(phydev, AT803X_MMD_ACCESS_CONTROL_DATA,
+ mac[(i * 2) + 1] | (mac[(i * 2)] << 8));
+ }
+}
+
+static int at803x_config_init(struct phy_device *phydev)
+{
+ int val;
+ u32 features;
+ int status;
+
+ features = SUPPORTED_TP | SUPPORTED_MII | SUPPORTED_AUI |
+ SUPPORTED_FIBRE | SUPPORTED_BNC;
+
+ val = phy_read(phydev, MII_BMSR);
+ if (val < 0)
+ return val;
+
+ if (val & BMSR_ANEGCAPABLE)
+ features |= SUPPORTED_Autoneg;
+ if (val & BMSR_100FULL)
+ features |= SUPPORTED_100baseT_Full;
+ if (val & BMSR_100HALF)
+ features |= SUPPORTED_100baseT_Half;
+ if (val & BMSR_10FULL)
+ features |= SUPPORTED_10baseT_Full;
+ if (val & BMSR_10HALF)
+ features |= SUPPORTED_10baseT_Half;
+
+ if (val & BMSR_ESTATEN) {
+ val = phy_read(phydev, MII_ESTATUS);
+ if (val < 0)
+ return val;
+
+ if (val & ESTATUS_1000_TFULL)
+ features |= SUPPORTED_1000baseT_Full;
+ if (val & ESTATUS_1000_THALF)
+ features |= SUPPORTED_1000baseT_Half;
+ }
+
+ phydev->supported = features;
+ phydev->advertising = features;
+
+ /* enable WOL */
+ at803x_set_wol_mac_addr(phydev);
+ status = phy_write(phydev, AT803X_INTR_ENABLE, AT803X_WOL_ENABLE);
+ status = phy_read(phydev, AT803X_INTR_STATUS);
+
+ return 0;
+}
+
+/* ATHEROS 8035 */
+static struct phy_driver at8035_driver = {
+ .phy_id = 0x004dd072,
+ .name = "Atheros 8035 ethernet",
+ .phy_id_mask = 0xffffffef,
+ .config_init = at803x_config_init,
+ .features = PHY_GBIT_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
+ .config_aneg = &genphy_config_aneg,
+ .read_status = &genphy_read_status,
+ .driver = {
+ .owner = THIS_MODULE,
+ },
+};
+
+/* ATHEROS 8030 */
+static struct phy_driver at8030_driver = {
+ .phy_id = 0x004dd076,
+ .name = "Atheros 8030 ethernet",
+ .phy_id_mask = 0xffffffef,
+ .config_init = at803x_config_init,
+ .features = PHY_GBIT_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
+ .config_aneg = &genphy_config_aneg,
+ .read_status = &genphy_read_status,
+ .driver = {
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init atheros_init(void)
+{
+ int ret;
+
+ ret = phy_driver_register(&at8035_driver);
+ if (ret)
+ goto err1;
+
+ ret = phy_driver_register(&at8030_driver);
+ if (ret)
+ goto err2;
+
+ return 0;
+
+err2:
+ phy_driver_unregister(&at8035_driver);
+err1:
+ return ret;
+}
+
+static void __exit atheros_exit(void)
+{
+ phy_driver_unregister(&at8035_driver);
+ phy_driver_unregister(&at8030_driver);
+}
+
+module_init(atheros_init);
+module_exit(atheros_exit);
+
+static struct mdio_device_id __maybe_unused atheros_tbl[] = {
+ { 0x004dd076, 0xffffffef },
+ { 0x004dd072, 0xffffffef },
+ { }
+};
+
+MODULE_DEVICE_TABLE(mdio, atheros_tbl);
--
1.7.9.5
^ permalink raw reply related
* Re: conntrack, NAT and icmp echo reply
From: Jozsef Kadlecsik @ 2012-10-12 12:41 UTC (permalink / raw)
To: Denys Fedoryshchenko; +Cc: Vigneswaran R, Eric Dumazet, netdev, netfilter
In-Reply-To: <4c082d4e3afe8226cb07baeeb550712d@visp.net.lb>
On Fri, 12 Oct 2012, Denys Fedoryshchenko wrote:
> On 2012-10-12 09:13, Vigneswaran R wrote:
> > On Thursday 11 October 2012 03:32 PM, Denys Fedoryshchenko wrote:
> > > On 2012-10-11 12:57, Eric Dumazet wrote:
> > > > On Thu, 2012-10-11 at 12:41 +0300, Denys Fedoryshchenko wrote:
> > > > >
> > > > > I have NAT box, with very simple rule
> > > > > iptables -t nat -I POSTROUTING -s 10.0.0.0/8 -j MASQUERADE
> > > > > It can be SNAT also, and it works fine, as NAT.
> > > > >
> > > > > When i generate icmp _reply_ packet, to some host
> > > > > hping -I ppp0 -1 --icmptype 0 8.8.8.8
> > > > >
> > > > > It will pass the box, and will exit it without NAT, e.g. with original
> > > > > IP 10.x.x.x
> > > > > on outgoing interface, which is not expected behavior IMHO.
> > > > > Is it a bug or feature?
> > > > >
> > > >
> > > > It depends, -s 10.0.0.0/8 wont match the rule if the source address
> > > > should be 198.23.44.55 I guess ?
> > > >
> > > > I would try the more obvious
> > > >
> > > > iptables -t nat -I POSTROUTING -o device -j MASQUERADE
> > > Source is correct, it is 10.0.0.0/8 range. I tested also ICMP code 3, it
> > > wont be NATed also.
> > > But ICMP echo passing OK.
> > > Also TCP RST generated same way, (i guess that don't have any match in
> > > conntrack table), won't be NATed too.
> > > hping -I ppp0 -R 8.8.8.8
> > > 13:01:07.074134 IP 10.0.0.142.2106 > 8.8.8.8.0: Flags [R], seq 510333079,
> > > win 512, length 0
> > > 13:01:08.074239 IP 10.0.0.142.2107 > 8.8.8.8.0: Flags [R], seq 1169580528,
> > > win 512, length 0
> > > 13:01:09.074253 IP 10.0.0.142.2108 > 8.8.8.8.0: Flags [R], seq 186548661,
> > > win 512, length 0
> > > 13:01:10.074376 IP 10.0.0.142.2109 > 8.8.8.8.0: Flags [R], seq 2135508128,
> > > win 512, length 0
> > > 13:01:11.074553 IP 10.0.0.142.2110 > 8.8.8.8.0: Flags [R], seq 1507433100,
> > > win 512, length 0
> > >
> > > And ICMP here you can see correct behavior with icmp echo request:
> > >
> > > 12:58:22.917458 IP 10.0.0.142 > 8.8.8.8: ICMP echo reply, id 62548, seq 0,
> > > length 8
> > > 12:58:23.917543 IP 10.0.0.142 > 8.8.8.8: ICMP echo reply, id 62548, seq
> > > 256, length 8
> > > 12:58:24.917657 IP 10.0.0.142 > 8.8.8.8: ICMP echo reply, id 62548, seq
> > > 512, length 8
> > > 12:58:31.047475 IP 10.0.0.142 > 8.8.8.8: ICMP net 5.6.7.8 unreachable,
> > > length 36
> > > 12:58:32.047562 IP 10.0.0.142 > 8.8.8.8: ICMP net 5.6.7.8 unreachable,
> > > length 36
> > > 12:58:33.047734 IP 10.0.0.142 > 8.8.8.8: ICMP net 5.6.7.8 unreachable,
> > > length 36
> > > 12:58:54.014601 IP X.146.153.X > 8.8.8.8: ICMP echo request, id 10462, seq
> > > 0, length 8
> > > 12:58:54.081897 IP 8.8.8.8 > X.146.153.X: ICMP echo reply, id 10462, seq
> > > 0, length 8
> >
> > I think, the following may be the reason for the behaviour you
> > observed. (I may be wrong, I am not an expert in iptables.)
> >
> > "nat" table only consulted for "NEW" connections. ref:
> > <http://inai.de/images/nf-packet-flow.svg>
> >
> > The ICMP echo _reply_ may not be considered as part of a "NEW"
> > connection, as it must be a _reply_ to some already received
> > _request_. So _request_ is new and _reply_ is not.
> >
> Yes, they are not related to existing and it is not new connection, but
> there is similar issues related to TCP, and it is handled differently.
> For example if nf_conntrack_tcp_loose is set to 0 - it will not pickup
> already established connections, and just they will have INVALID state.
> If set to 1 (and it is default value) - it will pickup the connection,
> even it is established state. I was expecting ICMP and especially RST
> can be consistent with that.
TCP is a connection-oriented procol while ICMP isn't. There is no point
in creating a connection entry from a bogus-looking ICMP reply packet.
The packets you generated neither belong nor related to an existing
connection, thus are marked as INVALID. INVALID packets are not NAT-ed and
if not dropped explicitely, will be sent out without natted.
Best regards,
Jozsef
-
E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
H-1525 Budapest 114, POB. 49, Hungary
^ permalink raw reply
* Re: alignment faults in 3.6
From: Russell King - ARM Linux @ 2012-10-12 12:24 UTC (permalink / raw)
To: Eric Dumazet
Cc: Arnd Bergmann, linux-arm-kernel, Måns Rullgård,
David Laight, netdev, Jon Masters
In-Reply-To: <1350036263.21172.11438.camel@edumazet-glaptop>
On Fri, Oct 12, 2012 at 12:04:23PM +0200, Eric Dumazet wrote:
> On Fri, 2012-10-12 at 10:03 +0100, Russell King - ARM Linux wrote:
>
> > No. It is my understanding that various IP option processing can also
> > cause the alignment fault handler to be invoked, even when the packet is
> > properly aligned, and then there's jffs2/mtd which also relies upon
> > alignment faults being fixed up.
>
> Oh well.
>
> We normally make sure we dont have alignment faults on arches that dont
> have CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS (or a non null NET_IP_ALIGN)
>
> So if you find an offender, please report a bug, because I can guarantee
> you we will _fix_ it.
I think one change I will make to the ARM alignment fixup is to get it
to record the last PC where a misaligned kernel fault occurred, and
report it via our statistics procfs file. That should allow us to
track down where some of these occur.
They aren't anywhere near regular though - looking at the statistics, my
firewall seems to do an average of around 2-3 a day, and a web server
around 7-8 a day.
^ permalink raw reply
* Re: alignment faults in 3.6
From: Måns Rullgård @ 2012-10-12 12:16 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Måns Rullgård, Arnd Bergmann, linux-arm-kernel,
David Laight, Eric Dumazet, netdev, Jon Masters
In-Reply-To: <20121012114443.GG21164@n2100.arm.linux.org.uk>
Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> On Fri, Oct 12, 2012 at 12:18:08PM +0100, Måns Rullgård wrote:
>> Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
>> > Well, I get the last word here and it's no.
>>
>> Sadly, yes.
>
> It's not "sadly" - it's a matter of fact that the kernel does from time
> to time generate misaligned accesses and they are _not_ bugs. If they
> were bugs, then the code to fix up misaligned accesses would not have
> been developed, and we'd instead take the fault and panic or something
> like that.
>
>> >> >> If all alignment faults in the kernel are caused by broken drivers,
>> >> >> that would at least give us some hope of finding those drivers while
>> >> >> at the same time not causing much overhead in the case where we need
>> >> >> to do the fixup in the meantime.
>> >> >
>> >> > No. It is my understanding that various IP option processing can also
>> >> > cause the alignment fault handler to be invoked, even when the packet is
>> >> > properly aligned, and then there's jffs2/mtd which also relies upon
>> >> > alignment faults being fixed up.
>> >>
>> >> As far as I'm concerned, this is all hearsay, and I've only ever heard
>> >> it from you. Why can't you let those who care fix these bugs instead?
>> >
>> > You know, I'm giving you the benefit of my _knowledge_ which has been
>> > built over the course of the last 20 years.
>>
>> How proud you sound. Now could you say something of substance instead?
>
> You're proving yourself to be idiot? There, that's substance.
>
>> > I've been in these discussions with networking people before. I ended
>> > up having to develop the alignment fault handler because of those
>> > discussions. And oh look, Eric confirmed that the networking code
>> > isn't going to get "fixed" as you were demanding, which is exactly
>> > what I said.
>>
>> Funny, I saw him say the exact opposite:
>>
>> So if you find an offender, please report a bug, because I can
>> guarantee you we will _fix_ it.
>
> No, let's go back.
>
> - You were demanding that the ipv4 header structure should be packed.
I demanded no such thing.
> I said that wasn't going to happen because the networking people
> wouldn't allow it, and it seems that's been proven correct.
> - You were demanding that the ipv4 code used the unaligned accessors.
Again, I made no such demand.
> I said that networking people wouldn't allow it either, and that's
> also been proven correct.
I did not, in fact, _demand_ anything at all. What I did say was that
to fix the problem of unaligned access traps the OP was having, the
(driver) code supplying the unaligned pointer should be fixed, or *if
that is not possible* mark the structs __packed. As it turns out,
fixing the driver is easy, so there is no need to change the structs or
how they are accessed.
> Both these points have been proven correct because Eric has said that the
> core networking code is _not_ going to be changed to suit this.
>
> What Eric _has_ said is that networking people consider packets supplied
> to the networking layer where the IPv4 header is not aligned on architectures
> where misaligned accesses are a problem to be a bug in the network driver,
> not the network code, and proposed a solution.
>
> That's entirely different from all your claims that the core networking
> code needs fixing to avoid these misaligned accesses.
I never said that. I said whatever code is responsible for the pointer
should be fixed. That code turns out to be the driver.
>> > I've been in discussions with MTD people over these issues before, I've
>> > discussed this with David Woodhouse when it came up in JFFS2. I *KNOW*
>> > these things.
>>
>> In the same way you "know" the networking people won't fix their code,
>> despite them _clearly_ stating the opposite?
>
> I'll tell you exactly how I *KNOW* this. The issue came up because of
> noMMU, which does not have any way to fix up alignment faults. JFFS2
> passes randomly aligned buffers to the MTD drivers, and the MTD drivers
> assume that they're aligned and they do word accesses on them.
So jffs2 is broken. Why can't it be fixed?
> See the thread http://lists.arm.linux.org.uk/lurker/thread/20021204.191632.4473796b.en.html
That thread is about detecting misaligned accesses and what to do with
them if they do occur. I don't see anyone successfully arguing against
fixing the code causing them.
> See: http://lists.arm.linux.org.uk/lurker/message/20020225.195925.02bdbd47.en.html
Yes, there seems to be a problem in jffs2. Or at least there was one 10
years ago.
> and: http://lists.arm.linux.org.uk/lurker/message/20020313.150932.081a7592.en.html
Yes, disabling the alignment trap on armv5 is a bad idea. Nobody has
argued otherwise.
> There's several other threads where it's also discussed.
>
> And while you're there, note the date. There is nothing recent about this
> issue. It's well known, and well understood by those who have a grasp of
> the issues that alignment faults are a part of normal operation by the
> ARM kernel - and is one of the penalties of being tied into architecture
> independent code.
>
> Compromises have to be sought, and that's the compromise we get to live
> with.
So because of some ancient history with jffs2, we should deny
present-day developers the tools to quickly identify problems in their
code? I just _love_ such compromises.
>> > You can call it hearsay if you wish, but it seems to be more accurate
>> > than your wild outlandish and pathetic statements.
>>
>> So you're resorting to name-calling. Not taking that bait.
>
> Sorry? So what you're saying is that it's fine for you to call my
> comments hearsay, but I'm not allowed to express a view on your comments.
> How arrogant of you.
Go ahead, pile it on. I'm not falling into this trap.
--
Måns Rullgård
mans@mansr.com
^ permalink raw reply
* Re: alignment faults in 3.6
From: Eric Dumazet @ 2012-10-12 12:08 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Måns Rullgård, Arnd Bergmann, linux-arm-kernel,
David Laight, netdev, Jon Masters
In-Reply-To: <20121012114443.GG21164@n2100.arm.linux.org.uk>
On Fri, 2012-10-12 at 12:44 +0100, Russell King - ARM Linux wrote:
> - You were demanding that the ipv4 header structure should be packed.
> I said that wasn't going to happen because the networking people
> wouldn't allow it, and it seems that's been proven correct.
> - You were demanding that the ipv4 code used the unaligned accessors.
> I said that networking people wouldn't allow it either, and that's
> also been proven correct.
>
> Both these points have been proven correct because Eric has said that the
> core networking code is _not_ going to be changed to suit this.
>
> What Eric _has_ said is that networking people consider packets supplied
> to the networking layer where the IPv4 header is not aligned on architectures
> where misaligned accesses are a problem to be a bug in the network driver,
> not the network code, and proposed a solution.
>
> That's entirely different from all your claims that the core networking
> code needs fixing to avoid these misaligned accesses.
It seems we agree then, but your words were a bit misleading (at least
for me)
So yes, we built network stack with the prereq that IP headers are
aligned, but unfortunately many developers use x86 which doesnt care, so
its possible some bugs are added.
But its not by intent, only by accident.
Thanks
^ permalink raw reply
* Re: [PATCH 0/3] virtio-net: inline header support
From: Cornelia Huck @ 2012-10-12 11:52 UTC (permalink / raw)
To: Rusty Russell
Cc: kvm, Michael S. Tsirkin, netdev, linux-kernel, virtualization,
Sasha Levin, Paolo Bonzini, avi, Thomas Lendacky
In-Reply-To: <87wqyw63et.fsf@rustcorp.com.au>
On Fri, 12 Oct 2012 09:07:46 +1030
Rusty Russell <rusty@rustcorp.com.au> wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > On Thu, Oct 11, 2012 at 10:33:31AM +1030, Rusty Russell wrote:
> >> OK. Well, Anthony wants qemu to be robust in this regard, so I am
> >> tempted to rework all the qemu drivers to handle arbitrary layouts.
> >> They could use a good audit anyway.
> >
> > I agree here. Still trying to understand whether we can agree to use
> > a feature bit for this, or not.
>
> I'd *like* to imply it by the new PCI layout, but if it doesn't work
> we'll add a new feature bit.
>
> I'm resisting a feature bit, since it constrains future implementations
> which could otherwise assume it.
>
> >> This would become a glaring exception, but I'm tempted to fix it to 32
> >> bytes at the same time as we get the new pci layout (ie. for the virtio
> >> 1.0 spec).
> >
> > But this isn't a virtio-pci only issue, is it?
> > qemu has s390 bus with same limmitation.
> > How can we tie it to pci layout?
>
> They can use a transport feature if they need to, of course. But
> perhaps the timing with ccw will coincide with the fix, in which they
> don't need to, but it might be a bit late.
>
> Cornelia?
My virtio-ccw host code is still going through a bit of rework, so it
might well go in after the fix.
There's also the existing (non-spec'ed) s390-virtio transport. While it
will likely be deprecated sometime in the future, it should probably
get a feature bit for consistency's sake.
>
> Cheers,
> Rusty.
>
^ permalink raw reply
* Re: alignment faults in 3.6
From: Russell King - ARM Linux @ 2012-10-12 11:44 UTC (permalink / raw)
To: Måns Rullgård
Cc: Arnd Bergmann, linux-arm-kernel, David Laight, Eric Dumazet,
netdev, Jon Masters
In-Reply-To: <yw1x8vbcndlb.fsf@unicorn.mansr.com>
On Fri, Oct 12, 2012 at 12:18:08PM +0100, Måns Rullgård wrote:
> Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> > Well, I get the last word here and it's no.
>
> Sadly, yes.
It's not "sadly" - it's a matter of fact that the kernel does from time
to time generate misaligned accesses and they are _not_ bugs. If they
were bugs, then the code to fix up misaligned accesses would not have
been developed, and we'd instead take the fault and panic or something
like that.
> >> >> If all alignment faults in the kernel are caused by broken drivers,
> >> >> that would at least give us some hope of finding those drivers while
> >> >> at the same time not causing much overhead in the case where we need
> >> >> to do the fixup in the meantime.
> >> >
> >> > No. It is my understanding that various IP option processing can also
> >> > cause the alignment fault handler to be invoked, even when the packet is
> >> > properly aligned, and then there's jffs2/mtd which also relies upon
> >> > alignment faults being fixed up.
> >>
> >> As far as I'm concerned, this is all hearsay, and I've only ever heard
> >> it from you. Why can't you let those who care fix these bugs instead?
> >
> > You know, I'm giving you the benefit of my _knowledge_ which has been
> > built over the course of the last 20 years.
>
> How proud you sound. Now could you say something of substance instead?
You're proving yourself to be idiot? There, that's substance.
> > I've been in these discussions with networking people before. I ended
> > up having to develop the alignment fault handler because of those
> > discussions. And oh look, Eric confirmed that the networking code
> > isn't going to get "fixed" as you were demanding, which is exactly
> > what I said.
>
> Funny, I saw him say the exact opposite:
>
> So if you find an offender, please report a bug, because I can
> guarantee you we will _fix_ it.
No, let's go back.
- You were demanding that the ipv4 header structure should be packed.
I said that wasn't going to happen because the networking people
wouldn't allow it, and it seems that's been proven correct.
- You were demanding that the ipv4 code used the unaligned accessors.
I said that networking people wouldn't allow it either, and that's
also been proven correct.
Both these points have been proven correct because Eric has said that the
core networking code is _not_ going to be changed to suit this.
What Eric _has_ said is that networking people consider packets supplied
to the networking layer where the IPv4 header is not aligned on architectures
where misaligned accesses are a problem to be a bug in the network driver,
not the network code, and proposed a solution.
That's entirely different from all your claims that the core networking
code needs fixing to avoid these misaligned accesses.
> > I've been in discussions with MTD people over these issues before, I've
> > discussed this with David Woodhouse when it came up in JFFS2. I *KNOW*
> > these things.
>
> In the same way you "know" the networking people won't fix their code,
> despite them _clearly_ stating the opposite?
I'll tell you exactly how I *KNOW* this. The issue came up because of
noMMU, which does not have any way to fix up alignment faults. JFFS2
passes randomly aligned buffers to the MTD drivers, and the MTD drivers
assume that they're aligned and they do word accesses on them.
See the thread http://lists.arm.linux.org.uk/lurker/thread/20021204.191632.4473796b.en.html
See: http://lists.arm.linux.org.uk/lurker/message/20020225.195925.02bdbd47.en.html
and: http://lists.arm.linux.org.uk/lurker/message/20020313.150932.081a7592.en.html
There's several other threads where it's also discussed.
And while you're there, note the date. There is nothing recent about this
issue. It's well known, and well understood by those who have a grasp of
the issues that alignment faults are a part of normal operation by the
ARM kernel - and is one of the penalties of being tied into architecture
independent code.
Compromises have to be sought, and that's the compromise we get to live
with.
> > You can call it hearsay if you wish, but it seems to be more accurate
> > than your wild outlandish and pathetic statements.
>
> So you're resorting to name-calling. Not taking that bait.
Sorry? So what you're saying is that it's fine for you to call my
comments hearsay, but I'm not allowed to express a view on your comments.
How arrogant of you.
^ permalink raw reply
* Re: alignment faults in 3.6
From: Russell King - ARM Linux @ 2012-10-12 11:19 UTC (permalink / raw)
To: Måns Rullgård
Cc: Eric Dumazet, Arnd Bergmann, netdev, David Laight, Jon Masters,
linux-arm-kernel
In-Reply-To: <20121012110750.GE21164@n2100.arm.linux.org.uk>
On Fri, Oct 12, 2012 at 12:07:50PM +0100, Russell King - ARM Linux wrote:
> On Fri, Oct 12, 2012 at 12:00:03PM +0100, Måns Rullgård wrote:
> > Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> >
> > > On Fri, Oct 12, 2012 at 08:11:42AM +0000, Arnd Bergmann wrote:
> > >> On Thursday 11 October 2012, Måns Rullgård wrote:
> > >> > > But, the IP header is expected to be aligned.
> > >> >
> > >> > Everything tells the compiler the struct is perfectly aligned. When the
> > >> > buggy driver passes a misaligned pointer, bad things happen.
> > >>
> > >> Would it be appropriate to add a WARN_ON_ONCE() in the alignment fault path
> > >> then?
> >
> > I think that's an excellent idea.
>
> Well, I get the last word here and it's no.
>
> > >> If all alignment faults in the kernel are caused by broken drivers,
> > >> that would at least give us some hope of finding those drivers while
> > >> at the same time not causing much overhead in the case where we need
> > >> to do the fixup in the meantime.
> > >
> > > No. It is my understanding that various IP option processing can also
> > > cause the alignment fault handler to be invoked, even when the packet is
> > > properly aligned, and then there's jffs2/mtd which also relies upon
> > > alignment faults being fixed up.
> >
> > As far as I'm concerned, this is all hearsay, and I've only ever heard
> > it from you. Why can't you let those who care fix these bugs instead?
>
> You know, I'm giving you the benefit of my _knowledge_ which has been
> built over the course of the last 20 years. I've been in these
> discussions with networking people before. I ended up having to develop
> the alignment fault handler because of those discussions.
Correction: San Mehat at Corel Inc had to develop it, but I was involved
in those discussions over it nevertheless, as I was the person who merged
the code and then subsequently maintained it.
^ permalink raw reply
* Re: alignment faults in 3.6
From: Måns Rullgård @ 2012-10-12 11:18 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Måns Rullgård, Arnd Bergmann, linux-arm-kernel,
David Laight, Eric Dumazet, netdev, Jon Masters
In-Reply-To: <20121012110750.GE21164@n2100.arm.linux.org.uk>
Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> On Fri, Oct 12, 2012 at 12:00:03PM +0100, Måns Rullgård wrote:
>> Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
>>
>> > On Fri, Oct 12, 2012 at 08:11:42AM +0000, Arnd Bergmann wrote:
>> >> On Thursday 11 October 2012, Måns Rullgård wrote:
>> >> > > But, the IP header is expected to be aligned.
>> >> >
>> >> > Everything tells the compiler the struct is perfectly aligned. When the
>> >> > buggy driver passes a misaligned pointer, bad things happen.
>> >>
>> >> Would it be appropriate to add a WARN_ON_ONCE() in the alignment
>> >> fault path then?
>>
>> I think that's an excellent idea.
>
> Well, I get the last word here and it's no.
Sadly, yes.
>> >> If all alignment faults in the kernel are caused by broken drivers,
>> >> that would at least give us some hope of finding those drivers while
>> >> at the same time not causing much overhead in the case where we need
>> >> to do the fixup in the meantime.
>> >
>> > No. It is my understanding that various IP option processing can also
>> > cause the alignment fault handler to be invoked, even when the packet is
>> > properly aligned, and then there's jffs2/mtd which also relies upon
>> > alignment faults being fixed up.
>>
>> As far as I'm concerned, this is all hearsay, and I've only ever heard
>> it from you. Why can't you let those who care fix these bugs instead?
>
> You know, I'm giving you the benefit of my _knowledge_ which has been
> built over the course of the last 20 years.
How proud you sound. Now could you say something of substance instead?
> I've been in these discussions with networking people before. I ended
> up having to develop the alignment fault handler because of those
> discussions. And oh look, Eric confirmed that the networking code
> isn't going to get "fixed" as you were demanding, which is exactly
> what I said.
Funny, I saw him say the exact opposite:
So if you find an offender, please report a bug, because I can
guarantee you we will _fix_ it.
> I've been in discussions with MTD people over these issues before, I've
> discussed this with David Woodhouse when it came up in JFFS2. I *KNOW*
> these things.
In the same way you "know" the networking people won't fix their code,
despite them _clearly_ stating the opposite?
> You can call it hearsay if you wish, but it seems to be more accurate
> than your wild outlandish and pathetic statements.
So you're resorting to name-calling. Not taking that bait.
--
Måns Rullgård
mans@mansr.com
^ permalink raw reply
* Re: alignment faults in 3.6
From: Russell King - ARM Linux @ 2012-10-12 11:07 UTC (permalink / raw)
To: Måns Rullgård
Cc: Arnd Bergmann, linux-arm-kernel, David Laight, Eric Dumazet,
netdev, Jon Masters
In-Reply-To: <yw1xhaq0nefg.fsf@unicorn.mansr.com>
On Fri, Oct 12, 2012 at 12:00:03PM +0100, Måns Rullgård wrote:
> Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
>
> > On Fri, Oct 12, 2012 at 08:11:42AM +0000, Arnd Bergmann wrote:
> >> On Thursday 11 October 2012, Måns Rullgård wrote:
> >> > > But, the IP header is expected to be aligned.
> >> >
> >> > Everything tells the compiler the struct is perfectly aligned. When the
> >> > buggy driver passes a misaligned pointer, bad things happen.
> >>
> >> Would it be appropriate to add a WARN_ON_ONCE() in the alignment fault path
> >> then?
>
> I think that's an excellent idea.
Well, I get the last word here and it's no.
> >> If all alignment faults in the kernel are caused by broken drivers,
> >> that would at least give us some hope of finding those drivers while
> >> at the same time not causing much overhead in the case where we need
> >> to do the fixup in the meantime.
> >
> > No. It is my understanding that various IP option processing can also
> > cause the alignment fault handler to be invoked, even when the packet is
> > properly aligned, and then there's jffs2/mtd which also relies upon
> > alignment faults being fixed up.
>
> As far as I'm concerned, this is all hearsay, and I've only ever heard
> it from you. Why can't you let those who care fix these bugs instead?
You know, I'm giving you the benefit of my _knowledge_ which has been
built over the course of the last 20 years. I've been in these
discussions with networking people before. I ended up having to develop
the alignment fault handler because of those discussions. And oh look,
Eric confirmed that the networking code isn't going to get "fixed" as
you were demanding, which is exactly what I said.
I've been in discussions with MTD people over these issues before, I've
discussed this with David Woodhouse when it came up in JFFS2. I *KNOW*
these things.
You can call it hearsay if you wish, but it seems to be more accurate
than your wild outlandish and pathetic statements.
^ permalink raw reply
* Re: alignment faults in 3.6
From: Måns Rullgård @ 2012-10-12 11:00 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Arnd Bergmann, linux-arm-kernel, Måns Rullgård,
David Laight, Eric Dumazet, netdev, Jon Masters
In-Reply-To: <20121012090321.GA21164@n2100.arm.linux.org.uk>
Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> On Fri, Oct 12, 2012 at 08:11:42AM +0000, Arnd Bergmann wrote:
>> On Thursday 11 October 2012, Måns Rullgård wrote:
>> > > But, the IP header is expected to be aligned.
>> >
>> > Everything tells the compiler the struct is perfectly aligned. When the
>> > buggy driver passes a misaligned pointer, bad things happen.
>>
>> Would it be appropriate to add a WARN_ON_ONCE() in the alignment fault path
>> then?
I think that's an excellent idea.
>> If all alignment faults in the kernel are caused by broken drivers,
>> that would at least give us some hope of finding those drivers while
>> at the same time not causing much overhead in the case where we need
>> to do the fixup in the meantime.
>
> No. It is my understanding that various IP option processing can also
> cause the alignment fault handler to be invoked, even when the packet is
> properly aligned, and then there's jffs2/mtd which also relies upon
> alignment faults being fixed up.
As far as I'm concerned, this is all hearsay, and I've only ever heard
it from you. Why can't you let those who care fix these bugs instead?
--
Måns Rullgård
mans@mansr.com
^ permalink raw reply
* Re: alignment faults in 3.6
From: Eric Dumazet @ 2012-10-12 10:04 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Arnd Bergmann, linux-arm-kernel, Måns Rullgård,
David Laight, netdev, Jon Masters
In-Reply-To: <20121012090321.GA21164@n2100.arm.linux.org.uk>
On Fri, 2012-10-12 at 10:03 +0100, Russell King - ARM Linux wrote:
> No. It is my understanding that various IP option processing can also
> cause the alignment fault handler to be invoked, even when the packet is
> properly aligned, and then there's jffs2/mtd which also relies upon
> alignment faults being fixed up.
Oh well.
We normally make sure we dont have alignment faults on arches that dont
have CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS (or a non null NET_IP_ALIGN)
So if you find an offender, please report a bug, because I can guarantee
you we will _fix_ it.
One example of a fix was the following subtle one.
commit 117632e64d2a5f464e491fe221d7169a3814a77b
tcp: take care of misalignments
We discovered that TCP stack could retransmit misaligned skbs if a
malicious peer acknowledged sub MSS frame. This currently can happen
only if output interface is non SG enabled : If SG is enabled, tcp
builds headless skbs (all payload is included in fragments), so the tcp
trimming process only removes parts of skb fragments, header stay
aligned.
Some arches cant handle misalignments, so force a head reallocation and
shrink headroom to MAX_TCP_HEADER.
Dont care about misaligments on x86 and PPC (or other arches setting
NET_IP_ALIGN to 0)
This patch introduces __pskb_copy() which can specify the headroom of
new head, and pskb_copy() becomes a wrapper on top of __pskb_copy()
^ permalink raw reply
* Re: alignment faults in 3.6
From: Russell King - ARM Linux @ 2012-10-12 9:03 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Måns Rullgård, David Laight,
Eric Dumazet, netdev, Jon Masters
In-Reply-To: <201210120811.43290.arnd@arndb.de>
On Fri, Oct 12, 2012 at 08:11:42AM +0000, Arnd Bergmann wrote:
> On Thursday 11 October 2012, Måns Rullgård wrote:
> > > But, the IP header is expected to be aligned.
> >
> > Everything tells the compiler the struct is perfectly aligned. When the
> > buggy driver passes a misaligned pointer, bad things happen.
>
> Would it be appropriate to add a WARN_ON_ONCE() in the alignment fault path
> then? If all alignment faults in the kernel are caused by broken drivers,
> that would at least give us some hope of finding those drivers while at the
> same time not causing much overhead in the case where we need to do the
> fixup in the meantime.
No. It is my understanding that various IP option processing can also
cause the alignment fault handler to be invoked, even when the packet is
properly aligned, and then there's jffs2/mtd which also relies upon
alignment faults being fixed up.
^ 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