* Re: [PATCH] fec: use interrupt for MDIO completion indication
From: David Miller @ 2010-07-22 21:12 UTC (permalink / raw)
To: w.sang; +Cc: baruch, netdev, bryan.wu, kernel, gerg, linux-arm-kernel
In-Reply-To: <20100721125113.GA2651@pengutronix.de>
From: Wolfram Sang <w.sang@pengutronix.de>
Date: Wed, 21 Jul 2010 14:51:13 +0200
> From: Wolfram Sang <w.sang@pengutronix.de>
> Subject: [PATCH] net/fec: restore interrupt mask after software-reset in fec_stop()
>
> After the change from mdio polling to irq, it became necessary to
> restore the interrupt mask after resetting the chip in fec_stop().
> Otherwise, with all irqs disabled, no communication with the PHY will be
> possible after e.g. un-/replugging the cable and the device gets
> stalled.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6 1/2] bonding: change test for presence of VLANs
From: David Miller @ 2010-07-22 21:12 UTC (permalink / raw)
To: fubar; +Cc: netdev, pedro.netdev, kaber
In-Reply-To: <1279750488-32611-1-git-send-email-fubar@us.ibm.com>
From: Jay Vosburgh <fubar@us.ibm.com>
Date: Wed, 21 Jul 2010 15:14:47 -0700
> After commit:
>
> commit ad1afb00393915a51c21b1ae8704562bf036855f
> Author: Pedro Garcia <pedro.netdev@dondevamos.com>
> Date: Sun Jul 18 15:38:44 2010 -0700
>
> vlan_dev: VLAN 0 should be treated as "no vlan tag" (802.1p packet)
>
> it is now regular practice for a VLAN "add vid" for VLAN 0 to
> arrive prior to any VLAN registration or creation of a vlan_group.
>
> This patch updates the bonding code that tests for the presence
> of VLANs configured above bonding. The new logic tests for bond->vlgrp
> to determine if a registration has occured, instead of testing that
> bonding's internal vlan_list is empty.
>
> The old code would panic when vlan_list was not empty, but
> vlgrp was still NULL (because only an "add vid" for VLAN 0 had occured).
>
> Bonding still adds VLAN 0 to its internal list so that 802.1p
> frames are handled correctly on transmit when non-VLAN accelerated
> slaves are members of the bond. The test against bond->vlan_list
> remains in bond_dev_queue_xmit for this reason.
>
> Modification to the bond->vlgrp now occurs under lock (in
> addition to RTNL), because not all inspections of it occur under RTNL.
>
> Additionally, because 8021q will never issue a "kill vid" for
> VLAN 0, there is now logic in bond_uninit to release any remaining
> entries from vlan_list.
>
> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
Applied.
^ permalink raw reply
* Re: [PATCH] au1000_eth: get ethernet address from platform_data
From: David Miller @ 2010-07-22 21:12 UTC (permalink / raw)
To: manuel.lauss; +Cc: linux-mips, netdev
In-Reply-To: <1279715450-28762-1-git-send-email-manuel.lauss@googlemail.com>
From: Manuel Lauss <manuel.lauss@googlemail.com>
Date: Wed, 21 Jul 2010 14:30:50 +0200
> au1000_eth uses firmware calls to get a valid MAC address, and changes
> it depending on platform device id. This patch moves this logic out
> of the driver into the platform device registration part, where boards
> with supported chips can use whatever firmware interface they need;
> the default implementation maintains compatibility with existing,
> YAMON-based firmware.
>
> Tested-by: Wolfgang Grandegger <wg@denx.de>
> Acked-by: Florian Fainelli <florian@openwrt.org>
> Signed-off-by: Manuel Lauss <manuel.lauss@googlemail.com>
> ---
> This patch depends on another patch to the MIPS tree to
> apply cleanly, so I'd prefer if it went in through it as well.
Ok, feel free:
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH] 3c59x: handle pci_iomap() errors
From: David Miller @ 2010-07-22 21:11 UTC (permalink / raw)
To: segooon
Cc: kernel-janitors, klassert, eric.dumazet, ben, adobriyan, joe,
netdev
In-Reply-To: <1279713638-31567-1-git-send-email-segooon@gmail.com>
From: Kulikov Vasiliy <segooon@gmail.com>
Date: Wed, 21 Jul 2010 16:00:36 +0400
> pci_iomap() can fail, handle this case and return -ENOMEM from probe
> function.
>
> Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
Applied.
We seem to leak the iomaps if vortex_probe1() fails, feel free to fix
that too :-)
^ permalink raw reply
* (Lack of) specification for RX n-tuple filtering
From: Ben Hutchings @ 2010-07-22 21:02 UTC (permalink / raw)
To: Peter Waskiewicz; +Cc: netdev, David Miller
The n-tuple filtering facility is half-baked at present. There is an
interface to add filters but none to remove them! And ETHTOOL_GRXNTUPLE
is not at all symmetric with ETHTOOL_SRXNTUPLE (which I complained about
at the time it was added, to no avail).
An ETHTOOL_RESET command with flag ETH_RESET_FILTER set could be defined
to clear all the filters, but that's a big hammer to use, and I think
that in general drivers should push the same configuration back to the
hardware after resetting it for whatever reason.
So far as I can work out, ixgbe clears all the filters when the filter
table fills up. Is that true? Is this really the intended behaviour of
manually set filters?
I also see this in the ixgbe implementation:
/*
* Program the relevant mask registers. If src/dst_port or src/dst_addr
* are zero, then assume a full mask for that field. Also assume that
* a VLAN of 0 is unspecified, so mask that out as well. L4type
* cannot be masked out in this implementation.
*
* This also assumes IPv4 only. IPv6 masking isn't supported at this
* point in time.
*/
An IPv4 address of 0 is certainly valid, so this isn't a good rule. And
in any case, such a rule should be specified *with the interface*, in
<linux/ethtool.h>, not the implementation.
This also implies that 'mask' specifies bits to be ignored, not bits to
be matched. That also was not specified.
Ben.`
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: RTA_MARK addition
From: David Miller @ 2010-07-22 20:46 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1279699394.2452.6.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 21 Jul 2010 10:03:14 +0200
> Add a new rt attribute, RTA_MARK, and use it in
> rt_fill_info()/inet_rtm_getroute() to support following commands :
>
> ip route get 192.168.20.110 mark NUMBER
> ip route get 192.168.20.108 from 192.168.20.110 iif eth1 mark NUMBER
> ip route list cache [192.168.20.110] mark NUMBER
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: With disable_ipv6 set to 1 on an interface, ff00:/8 and fe80::/64 are still added on device UP
From: David Miller @ 2010-07-22 20:41 UTC (permalink / raw)
To: brian.haley; +Cc: maheshkelkar, netdev
In-Reply-To: <4C460856.5090701@hp.com>
From: Brian Haley <brian.haley@hp.com>
Date: Tue, 20 Jul 2010 16:34:30 -0400
> If the interface has IPv6 disabled, don't add a multicast or
> link-local route since we won't be adding a link-local address.
>
> Reported-by: Mahesh Kelkar <maheshkelkar@gmail.com>
> Signed-off-by: Brian Haley <brian.haley@hp.com>
Applied, thanks Brian.
^ permalink raw reply
* Re: [PATCH] Fix corruption of skb csum field in pskb_expand_head() of net/core/skbuff.c
From: David Miller @ 2010-07-22 20:28 UTC (permalink / raw)
To: andrea; +Cc: linux-kernel, netdev
In-Reply-To: <20100722191234.GA832@cronus.persephoneslair.org>
From: Andrea Shepard <andrea@persephoneslair.org>
Date: Thu, 22 Jul 2010 12:12:35 -0700
> Make pskb_expand_head() check ip_summed to make sure csum_start is really
> csum_start and not csum before adjusting it.
...
> Signed-off-by: Andrea Shepard <andrea@persephoneslair.org>
Applied, but your email client corrupted tab characters into spaces so
I had to apply your patch by hand.
There is a similar bug in skb_copy_expand() so I fixed that too.
Thanks again.
--------------------
net: Fix skb_copy_expand() handling of ->csum_start
It should only be adjusted if ip_summed == CHECKSUM_PARTIAL.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/core/skbuff.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index c699159..ce88293 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -932,7 +932,8 @@ struct sk_buff *skb_copy_expand(const struct sk_buff *skb,
copy_skb_header(n, skb);
off = newheadroom - oldheadroom;
- n->csum_start += off;
+ if (n->ip_summed == CHECKSUM_PARTIAL)
+ n->csum_start += off;
#ifdef NET_SKBUFF_DATA_USES_OFFSET
n->transport_header += off;
n->network_header += off;
--
1.7.1.1
^ permalink raw reply related
* Re: macvtap: Limit packet queue length
From: David Miller @ 2010-07-22 20:09 UTC (permalink / raw)
To: arnd; +Cc: herbert, netdev, mwagner, chrisw
In-Reply-To: <201007221130.53612.arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 22 Jul 2010 11:30:53 +0200
> On Thursday 22 July 2010, Herbert Xu wrote:
>> Reported-by: Mark Wagner <mwagner@redhat.com>
>> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> As long as we're missing a better solution,
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
Applied, thanks everyone.
^ permalink raw reply
* [PATCH] minstrel: don't complain about feedback for unrequested rates
From: John W. Linville @ 2010-07-22 19:46 UTC (permalink / raw)
To: linux-wireless; +Cc: Felix Fietkau, Sven Geggus, netdev, John W. Linville
In-Reply-To: <4C4896D5.8010802@openwrt.org>
"It's not problematic if minstrel gets feedback for rates that it
doesn't have in its list, it should just ignore it. - Felix"
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Cc: Felix Fietkau <nbd@openwrt.org>
---
net/mac80211/rc80211_minstrel.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
index f65ce6d..778c604 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -67,7 +67,6 @@ rix_to_ndx(struct minstrel_sta_info *mi, int rix)
for (i = rix; i >= 0; i--)
if (mi->r[i].rix == rix)
break;
- WARN_ON(i < 0);
return i;
}
--
1.7.1.1
^ permalink raw reply related
* Re: macvtap: Limit packet queue length
From: David Miller @ 2010-07-22 19:58 UTC (permalink / raw)
To: herbert; +Cc: mashirle, netdev, arnd, mwagner
In-Reply-To: <20100722160731.GA30723@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 23 Jul 2010 00:07:31 +0800
> On Thu, Jul 22, 2010 at 08:59:58AM -0700, Shirley Ma wrote:
>> On Thu, 2010-07-22 at 14:41 +0800, Herbert Xu wrote:
>> > {
>> > struct macvtap_queue *q = macvtap_get_queue(dev, skb);
>> > if (!q)
>> > - return -ENOLINK;
>> > + goto drop;
>> > +
>> > + if (skb_queue_len(&q->sk.sk_receive_queue) >=
>> > dev->tx_queue_len)
>> > + goto drop;
>> >
>>
>> Do we need to orphan skb here, just like tun?
>
> We could, but that is orthogonal to the problem at hand so feel
> free to do that in another patch.
These days, the stack pre-orphans all packets sent to ->ndo_start_xmit()
in dev_hard_start_xmit() as long as socket based TX timestamping is not
active for the packet.
^ permalink raw reply
* [net 2/2] veth: Remove redundant timestamp assignment.
From: Ben Greear @ 2010-07-22 19:54 UTC (permalink / raw)
To: netdev; +Cc: Ben Greear
In-Reply-To: <1279828488-17800-1-git-send-email-greearb@candelatech.com>
The timestamp is already cleared in the dev_forward_skb
logic.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 0cfbb3d... 161ee02... M drivers/net/veth.c
drivers/net/veth.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 0cfbb3d..161ee02 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -181,11 +181,6 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
if (dev->features & NETIF_F_NO_CSUM)
skb->ip_summed = rcv_priv->ip_summed;
- /* Zero out the time-stamp so that receiving code is forced
- * to recalculate it.
- */
- skb->tstamp.tv64 = 0;
-
length = skb->len + ETH_HLEN;
if (dev_forward_skb(rcv, skb) != NET_RX_SUCCESS)
goto rx_drop;
--
1.6.2.5
^ permalink raw reply related
* [net 1/2] net: dev_forward_skb should call nf_reset
From: Ben Greear @ 2010-07-22 19:54 UTC (permalink / raw)
To: netdev; +Cc: Ben Greear
In-Reply-To: <1279828488-17800-1-git-send-email-greearb@candelatech.com>
With conn-track zones and probably with different network
namespaces, the netfilter logic needs to be re-calculated
on packet receive. If the netfilter logic is not reset,
it will not be recalculated properly. This patch adds
the nf_reset logic to dev_forward_skb.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 7ac33e5... 22eee4e... M net/core/dev.c
net/core/dev.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 7ac33e5..22eee4e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1482,6 +1482,7 @@ static inline void net_timestamp(struct sk_buff *skb)
int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
{
skb_orphan(skb);
+ nf_reset(skb);
if (!(dev->flags & IFF_UP) ||
(skb->len > (dev->mtu + dev->hard_header_len))) {
--
1.6.2.5
^ permalink raw reply related
* [net 0/2] Clean up netfilter cache on xmit-to-self.
From: Ben Greear @ 2010-07-22 19:54 UTC (permalink / raw)
To: netdev
These patches are against 2.6.34.1. The first one may be
worth submitting to stable. The second one is less critical
and should probably just go in the development tree.
^ permalink raw reply
* Re: [RFC 1/1] netfilter: xtables: inclusion of xt_condition
From: Luciano Coelho @ 2010-07-22 19:30 UTC (permalink / raw)
To: ext Alexey Dobriyan
Cc: Jan Engelhardt, Netfilter Developer Mailing List,
netdev@vger.kernel.org, Patrick McHardy, sameo@linux.intel.com
In-Reply-To: <20100722191940.GA10029@x200>
On Thu, 2010-07-22 at 21:19 +0200, ext Alexey Dobriyan wrote:
> On Thu, Jul 22, 2010 at 04:44:35PM +0200, Jan Engelhardt wrote:
> >
> > On Thursday 2010-07-22 16:09, Luciano Coelho wrote:
> > >+static int condition_mt_check(const struct xt_mtchk_param *par)
> > >+{
> > >+ struct xt_condition_mtinfo *info = par->matchinfo;
> > >+ struct condition_variable *var;
> > >+ struct condition_net *cond_net =
> > >+ condition_pernet(current->nsproxy->net_ns);
> >
> > Cc'ing Alexey who has done the netns support.
> >
> > Alexey, you added par->net, but given Luciano just did it with
> > current->nsproxy->net_ns, do we really need par->net?
>
> In ->check, maybe, we can get away with current->nsproxy->net_ns.
>
> But definitely not in ->destroy(), because destruction can happen
> when _no_ task is in netns, so current->nsproxy->net_ns is 100% bogus.
>
> Steps to reproduce:
> iptables -A ...
> exit
>
> ->destroy hook gets netns from par->net, ->checkentry does the same
> for symmetry and less confusion.
Very good point. I guess that when Patrick suggested using
current->nsproxy->net_ns, he meant only for the module_params part.
I'll be removing that anyway. And I'll change the code to use par->net
instead of current->nsproxy->net_ns to avoid the problem in _destroy.
Thanks for your comments!
I must admit that I was a bit insecure about this code. That's why I
sent a RFC early enough. ;)
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH] CAN: Add Flexcan CAN controller driver
From: David Miller @ 2010-07-22 19:29 UTC (permalink / raw)
To: mkl; +Cc: wg, socketcan-core, netdev, s.hauer
In-Reply-To: <1279816844-12138-1-git-send-email-mkl@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Thu, 22 Jul 2010 18:40:44 +0200
> The following changes since commit 4cfa580e7eebb8694b875d2caff3b989ada2efac:
>
> r6040: Fix args to phy_mii_ioctl(). (2010-07-21 21:10:49 -0700)
>
> are available in the git repository at:
> git://git.pengutronix.de/git/mkl/linux-2.6.git for-net-next-2.6
>
> Marc Kleine-Budde (1):
> CAN: Add Flexcan CAN controller driver
Pulled, thanks.
^ permalink raw reply
* Re: [Uclinux-dist-devel] [PATCH 2/2] net: dsa: introduce MICREL KSZ8893MQL/BL ethernet switch chip support
From: Karl Beldan @ 2010-07-22 19:27 UTC (permalink / raw)
To: Mike Frysinger
Cc: Lennert Buytenhek, netdev, uclinux-dist-devel, David S. Miller
In-Reply-To: <AANLkTikoSb8WA40teXaYvaPf1XsC4r7ySWZOBTKug4ip@mail.gmail.com>
On Wed, Jul 21, 2010 at 11:00 PM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> On Wed, Jul 21, 2010 at 16:31, Karl Beldan wrote:
>> On Wed, Jul 21, 2010 at 6:05 PM, Mike Frysinger wrote:
>>> On Wed, Jul 21, 2010 at 11:16, Lennert Buytenhek wrote:
>>>> On Wed, Jul 21, 2010 at 09:37:22AM -0400, Mike Frysinger wrote:
>>>>> +static int ksz8893m_setup(struct dsa_switch *ds)
>>>>> +{
>>>>> + int i;
>>>>> + int ret;
>>>>> +
>>>>> + ret = ksz8893m_switch_reset(ds);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>
>>>> It's pretty ugly that the mdiobus is passed in via the normal means,
>>>> but a reference to the SPI bus to use is just stuffed into some global
>>>> variable.
>>>>
>>>> Can you not access all registers via MII?
>>>
>>> it depends on the host mdio bus. if it supports the semi-standard
>>> behavior of toggling the OP field of MDIO frames, then yes, you can do
>>> it via MII. but i dont think the current mdio framework in the kernel
>>> keeps track of that functionality, so there isnt a way in the driver
>>> to say "is this possible, else fall back to SPI".
>>
>> Are you referring to SMI ?
>
> that is the term the ksz datasheet uses (Serial Management Interface),
> but i'm not aware of it being a standardized term. even the ksz
> datasheet admits that its behavior is a bit non-standard. the
> Blackfin MAC doesnt support it so we have no way of testing the it as
> that is the MAC on the board with the ksz switch.
>
Can't either, we don't even have mdio, only spi.
Speaking of which, maybe you can replace this while you are at it :
--- net/dsa/slave.c 2010-05-22 09:58:34.000000000 +0200
void dsa_slave_mii_bus_init(struct dsa_switch *ds)
{
ds->slave_mii_bus->priv = (void *)ds;
- ds->slave_mii_bus->name = "dsa slave smi";
--
Karl
^ permalink raw reply
* Re: [RFC 1/1] netfilter: xtables: inclusion of xt_condition
From: Luciano Coelho @ 2010-07-22 19:26 UTC (permalink / raw)
To: ext Jan Engelhardt
Cc: Netfilter Developer Mailing List, netdev@vger.kernel.org,
Patrick McHardy, sameo@linux.intel.com, Alexey Dobriyan
In-Reply-To: <alpine.LSU.2.01.1007221733360.12308@obet.zrqbmnf.qr>
On Thu, 2010-07-22 at 17:36 +0200, ext Jan Engelhardt wrote:
> On Thursday 2010-07-22 17:16, Luciano Coelho wrote:
> >> >+ ret = strict_strtoul(val, 0, &l);
> >> >+ if (ret == -EINVAL || ((uint)l != l))
> >> >+ return -EINVAL;
> >>
> >> >+ *((u32 *) ((u8 *) cond_net + (size_t) kp->arg)) = l;
> >>
> >> I don't think we need this level of granularity; let the options be
> >> global, similar to what xt_hashlimit does.
> >
> >I did this according to Patrick's comment:
> >> > proc_net_condition is a global variable, so this won't work for
> >> > namespaces. What the code does is reinitialize it when instantiating
> >> > a new namespace, so it will always point to the last instantiated
> >> > namespace.
> >> >
> >> > The same problem exists for the condition_list, each namespace
> >> > should only be able to access its own conditions.
> >>
> >> This also applies to the permission variables. Basically, we shouldn't
> >> be having any globals except perhaps the mutex. You probably need a
> >> module_param_call function to set them for the correct namespace (you
> >> can access that through current->nsproxy->net_ns).
> >
> >I found it a bit strange to be able to change the module params in a
> >per-netns basis, but it is actually possible if you're changing the
> >parameters via sysfs. I tried it and it even seems to work. ;)
> >
> >I can't see any module parameters in the xt_hashlimit.c file. Am I
> >looking in the wrong place?
>
> Oops, xt_recent.c.
>
> >I would be fine with making the module params global (as they were
> >before), if that's fine with Patrick too.
>
> "When was the last time you needed to change the default ownership
> when you _also_ have the possibility to chown each procfs file
> individually?"
>
> >> (I am not even sure if kp->arg can be non-multiples-of-4, in which case
> >> this would be an alignment violation even.)
> >
> >I'm passing size_t in kp->arg. It looks quite ugly, because usually
> >kp->arg is a pointer to some data. But at least this way, using
> >offsetof(), I could avoid lots of repeated code for the options...
>
> if kp->arg is 1, ((u8*)cond_net + kp->arg) yields a pointer that's
> usually not aligned for u32. (And C pedants would probably argue
> that is should be char* not u8*, even if the one is a typedef
> of another.)
Oh, I see. In the current case it works because all the parameters are
32-bit.
In any case, I'll just remove the per-net module parameters code. It is
indeed over-fine-grained as you said earlier.
--
Cheers,
Luca.
^ permalink raw reply
* [PATCH] Fix corruption of skb csum field in pskb_expand_head() of net/core/skbuff.c
From: Andrea Shepard @ 2010-07-22 19:12 UTC (permalink / raw)
To: davem; +Cc: linux-kernel, netdev
Make pskb_expand_head() check ip_summed to make sure csum_start is really
csum_start and not csum before adjusting it.
This fixes a bug I encountered using a Sun Quad-Fast Ethernet card and VLANs.
On my configuration, the sunhme driver produces skbs with differing amounts
of headroom on receive depending on the packet size. See line 2030 of
drivers/net/sunhme.c; packets smaller than RX_COPY_THRESHOLD have 52 bytes
of headroom but packets larger than that cutoff have only 20 bytes.
When these packets reach the VLAN driver, vlan_check_reorder_header()
calls skb_cow(), which, if the packet has less than NET_SKB_PAD (== 32) bytes
of headroom, uses pskb_expand_head() to make more.
Then, pskb_expand_head() needs to adjust a lot of offsets into the skb,
including csum_start. Since csum_start is a union with csum, if the packet
has a valid csum value this will corrupt it, which was the effect I observed.
The sunhme hardware computes receive checksums, so the skbs would be created
by the driver with ip_summed == CHECKSUM_COMPLETE and a valid csum field, and
then pskb_expand_head() would corrupt the csum field, leading to an "hw csum
error" message later on, for example in icmp_rcv() for pings larger than the
sunhme RX_COPY_THRESHOLD.
On the basis of the comment at the beginning of include/linux/skbuff.h,
I believe that the csum_start skb field is only meaningful if ip_csummed is
CSUM_PARTIAL, so this patch makes pskb_expand_head() adjust it only in that
case to avoid corrupting a valid csum value.
Please see my more in-depth disucssion of tracking down this bug for
more details if you like:
http://puellavulnerata.livejournal.com/112186.html
http://puellavulnerata.livejournal.com/112567.html
http://puellavulnerata.livejournal.com/112891.html
http://puellavulnerata.livejournal.com/113096.html
http://puellavulnerata.livejournal.com/113591.html
I am not subscribed to this list, so please CC me on replies.
Signed-off-by: Andrea Shepard <andrea@persephoneslair.org>
diff -Nau linux-2.6.34.1/net/core/skbuff.c linux-2.6.34.1-skb-csum/net/core/skbuff.c
--- linux-2.6.34.1/net/core/skbuff.c 2010-07-05 11:24:10.000000000 -0700
+++ linux-2.6.34.1-skb-csum/net/core/skbuff.c 2010-07-21 18:42:33.000000000 -0700
@@ -854,7 +854,9 @@
skb->network_header += off;
if (skb_mac_header_was_set(skb))
skb->mac_header += off;
- skb->csum_start += nhead;
+ /* Only adjust this if it actually is csum_start rather than csum */
+ if (skb->ip_summed == CHECKSUM_PARTIAL)
+ skb->csum_start += nhead;
skb->cloned = 0;
skb->hdr_len = 0;
skb->nohdr = 0;
--
Andrea Shepard
^ permalink raw reply
* Re: [RFC 1/1] netfilter: xtables: inclusion of xt_condition
From: Alexey Dobriyan @ 2010-07-22 19:19 UTC (permalink / raw)
To: Jan Engelhardt
Cc: Luciano Coelho, Netfilter Developer Mailing List, netdev,
Patrick McHardy, sameo
In-Reply-To: <alpine.LSU.2.01.1007221621270.1619@obet.zrqbmnf.qr>
On Thu, Jul 22, 2010 at 04:44:35PM +0200, Jan Engelhardt wrote:
>
> On Thursday 2010-07-22 16:09, Luciano Coelho wrote:
> >+static int condition_mt_check(const struct xt_mtchk_param *par)
> >+{
> >+ struct xt_condition_mtinfo *info = par->matchinfo;
> >+ struct condition_variable *var;
> >+ struct condition_net *cond_net =
> >+ condition_pernet(current->nsproxy->net_ns);
>
> Cc'ing Alexey who has done the netns support.
>
> Alexey, you added par->net, but given Luciano just did it with
> current->nsproxy->net_ns, do we really need par->net?
In ->check, maybe, we can get away with current->nsproxy->net_ns.
But definitely not in ->destroy(), because destruction can happen
when _no_ task is in netns, so current->nsproxy->net_ns is 100% bogus.
Steps to reproduce:
iptables -A ...
exit
->destroy hook gets netns from par->net, ->checkentry does the same
for symmetry and less confusion.
^ permalink raw reply
* [PATCH ethtool 2/2] ethtool: Remove specification of number bases for RX-ntuple parameters
From: Ben Hutchings @ 2010-07-22 19:12 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, linux-net-drivers
In-Reply-To: <1279825899.2104.28.camel@achroite.uk.solarflarecom.com>
The numeric parameters for RX n-tuple filters may be specified as
either decimal or hexadecimal with a '0x' prefix, like most other
numeric parameters.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
ethtool.8 | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/ethtool.8 b/ethtool.8
index d0cbc93..98b57fb 100644
--- a/ethtool.8
+++ b/ethtool.8
@@ -689,28 +689,28 @@ Includes the destination IP address.
Specify a mask for the destination IP address.
.TP
.BI src-port \ port
-Includes the source port, specified in decimal.
+Includes the source port.
.TP
.BI src-port-mask \ mask
-Specify a mask for the source port, specified in hex.
+Specify a mask for the source port.
.TP
.BI dst-port \ port
-Includes the destination port, specified in decimal.
+Includes the destination port.
.TP
.BI dst-port-mask \ mask
-Specify a mask for the destination port, specified in hex.
+Specify a mask for the destination port.
.TP
.BI vlan \ VLAN-tag
-Includes the VLAN tag, specified in hex.
+Includes the VLAN tag.
.TP
.BI vlan-mask \ mask
-Specify a mask for the VLAN tag, specified in hex.
+Specify a mask for the VLAN tag.
.TP
.BI user-def \ data
-Includes 64-bits of user-specific data, specified in hex.
+Includes 64-bits of user-specific data.
.TP
.BI user-def-mask \ mask
-Specify a mask for the user-specific data, specified in hex.
+Specify a mask for the user-specific data.
.TP
.BI action \ N
Specifies either the Rx queue to send packets to, or to drop
--
1.6.2.5
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 related
* [PATCH ethtool 1/2] ethtool: Use inet_aton() to parse IPv4 addresses for RX n-tuple control
From: Ben Hutchings @ 2010-07-22 19:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, linux-net-drivers
Note that inet_aton() allows the address to be specified as a single
32-bit number in the same formats as strtoull(), so this is backward-
compatible.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
ethtool.8 | 9 +++++----
ethtool.c | 25 +++++++++++++++----------
2 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/ethtool.8 b/ethtool.8
index b0b3c8d..d0cbc93 100644
--- a/ethtool.8
+++ b/ethtool.8
@@ -676,16 +676,17 @@ Configure Rx ntuple filters and actions
.RE
.TP
.BI src-ip \ addr
-Includes the source IP address, specified in hex.
+Includes the source IP address, specified using dotted-quad notation
+or as a single 32-bit number.
.TP
.BI src-ip-mask \ mask
-Specify a mask for the source IP address, specified in hex.
+Specify a mask for the source IP address.
.TP
.BI dst-ip \ addr
-Includes the destination IP address, specified in hex.
+Includes the destination IP address.
.TP
.BI dst-ip-mask \ mask
-Specify a mask for the destination IP address, specified in hex.
+Specify a mask for the destination IP address.
.TP
.BI src-port \ port
Includes the source port, specified in decimal.
diff --git a/ethtool.c b/ethtool.c
index 4ab1a41..eef76f9 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -40,6 +40,10 @@
#include <limits.h>
#include <ctype.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+
#include <linux/sockios.h>
#include "ethtool-util.h"
@@ -404,7 +408,7 @@ typedef enum {
CMDL_U32,
CMDL_U64,
CMDL_BE16,
- CMDL_BE32,
+ CMDL_IP4,
CMDL_STR,
CMDL_FLAG,
} cmdline_type_t;
@@ -412,7 +416,7 @@ typedef enum {
struct cmdline_info {
const char *name;
cmdline_type_t type;
- /* Points to int (BOOL), s32, u16, u32 (U32/FLAG), u64 or
+ /* Points to int (BOOL), s32, u16, u32 (U32/FLAG/IP4), u64 or
* char * (STR). For FLAG, the value accumulates all flags
* to be set. */
void *wanted_val;
@@ -497,10 +501,10 @@ static struct cmdline_info cmdline_coalesce[] = {
};
static struct cmdline_info cmdline_ntuple[] = {
- { "src-ip", CMDL_BE32, &ntuple_fs.h_u.tcp_ip4_spec.ip4src, NULL },
- { "src-ip-mask", CMDL_BE32, &ntuple_fs.m_u.tcp_ip4_spec.ip4src, NULL },
- { "dst-ip", CMDL_BE32, &ntuple_fs.h_u.tcp_ip4_spec.ip4dst, NULL },
- { "dst-ip-mask", CMDL_BE32, &ntuple_fs.m_u.tcp_ip4_spec.ip4dst, NULL },
+ { "src-ip", CMDL_IP4, &ntuple_fs.h_u.tcp_ip4_spec.ip4src, NULL },
+ { "src-ip-mask", CMDL_IP4, &ntuple_fs.m_u.tcp_ip4_spec.ip4src, NULL },
+ { "dst-ip", CMDL_IP4, &ntuple_fs.h_u.tcp_ip4_spec.ip4dst, NULL },
+ { "dst-ip-mask", CMDL_IP4, &ntuple_fs.m_u.tcp_ip4_spec.ip4dst, NULL },
{ "src-port", CMDL_BE16, &ntuple_fs.h_u.tcp_ip4_spec.psrc, NULL },
{ "src-port-mask", CMDL_BE16, &ntuple_fs.m_u.tcp_ip4_spec.psrc, NULL },
{ "dst-port", CMDL_BE16, &ntuple_fs.h_u.tcp_ip4_spec.pdst, NULL },
@@ -645,11 +649,12 @@ static void parse_generic_cmdline(int argc, char **argp,
0xffff));
break;
}
- case CMDL_BE32: {
+ case CMDL_IP4: {
u32 *p = info[idx].wanted_val;
- *p = cpu_to_be32(
- get_uint_range(argp[i], 0,
- 0xffffffff));
+ struct in_addr in;
+ if (!inet_aton(argp[i], &in))
+ show_usage(1);
+ *p = in.s_addr;
break;
}
case CMDL_FLAG: {
--
1.6.2.5
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 related
* Re: minstrel_tx_status mac80211 WARNINGs in vanilla 2.6.34.1
From: Felix Fietkau @ 2010-07-22 19:07 UTC (permalink / raw)
To: John W. Linville
Cc: Sven Geggus, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20100722144723.GA2616-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
On 2010-07-22 4:47 PM, John W. Linville wrote:
> On Thu, Jul 22, 2010 at 02:30:01PM +0200, Sven Geggus wrote:
>> Hello,
>>
>> running vanilla 2.6.34.1 I get the following warnings in kernel log:
>>
>> WARNING: at net/mac80211/rc80211_minstrel.c:70 minstrel_tx_status+0x67/0xd1 [mac80211]()
>> Hardware name: SCENIC E300/E600
>> Modules linked in: i915 drm_kms_helper drm video backlight output lp loop
>> snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm_oss snd_mixer_oss snd_pcm
>> snd_seq_dummy zd1211rw snd_seq_oss usbhid mac80211 option cfg80211
>> snd_seq_midi usbserial snd_rawmidi snd_seq_midi_event snd_seq snd_timer
>> snd_seq_device snd parport_pc ehci_hcd uhci_hcd soundcore intel_agp parport
>> usbcore nls_base snd_page_alloc agpgart rng_core floppy sg
>> Pid: 0, comm: swapper Tainted: G W 2.6.34.1 #3
>> Call Trace:
>> [<c102af25>] warn_slowpath_common+0x60/0x90
>> [<c102af62>] warn_slowpath_null+0xd/0x10
>> [<f83cbd48>] minstrel_tx_status+0x67/0xd1 [mac80211]
>> [<f83b6eb1>] ieee80211_tx_status+0x1f6/0x5ac [mac80211]
>> [<c1261533>] ? skb_dequeue+0x45/0x4c
>> [<f83b6896>] ieee80211_tasklet_handler+0x61/0xd6 [mac80211]
>> [<c102ed7d>] tasklet_action+0x62/0x9f
>> [<c102f331>] __do_softirq+0x77/0xe5
>> [<c102f3c5>] do_softirq+0x26/0x2b
>> [<c102f52f>] irq_exit+0x29/0x66
>> [<c1003e90>] do_IRQ+0x85/0x9b
>> [<c1002d29>] common_interrupt+0x29/0x30
>> [<c10083ac>] ? default_idle+0x2d/0x42
>> [<c1001a9b>] cpu_idle+0x44/0x71
>> [<c12e00de>] rest_init+0x96/0x98
>> [<c1498862>] start_kernel+0x2a5/0x2aa
>> [<c14980b7>] i386_start_kernel+0xb7/0xbf
>> ---[ end trace f22ceacef336878f ]---
>>
>> Wireless driver is zd1211rw.
>>
>> Did not test with older kernel because this device has not been in user on
>> this machine before.
>>
>> WLAN does however seem to work anyway.
>
> Well, I just took a quick look -- so, I'm not 100% sure...
>
> But, it looks to me like zd1211rw is reporting tx status on a rate
> that minstrel didn't expect it to use. It seems like the hardware
> is pre-wired to do retries on sequentially lower rates, which seems
> a bit incompatible with minstrel's worldview.
>
> Felix, can we accomodate this? The "WLAN does however seem to work
> anyway" seems to suggest things work, so can we at least not yell
> about it?
I think we should just drop that WARN_ON(). It's not problematic if
minstrel gets feedback for rates that it doesn't have in its list, it
should just ignore it.
- Felix
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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
* Re: [PATCH V3] Export SMBIOS provided firmware instance and label to sysfs
From: Greg KH @ 2010-07-22 18:56 UTC (permalink / raw)
To: Narendra K
Cc: netdev, linux-hotplug, linux-pci, matt_domsch, charles_rose,
jordan_hargrave, vijay_nijhawan
In-Reply-To: <20100722184448.GA9122@auslistsprd01.us.dell.com>
On Thu, Jul 22, 2010 at 01:44:48PM -0500, Narendra K wrote:
> Hello,
>
> Resubmitting the patch with suggested changes -
>
> V2 -> V3:
>
> 1. Added documentation about the sysfs attributes in Documentation/ABI
> directory.
> 2. Changed the type of parameter 'attribute' of the function
> 'find_smbios_instance_string' from int to enum.
> 3. Changed the return type of the functions 'pci_create_firmware_label_files'
> and 'pci_remove_firmware_label_files' from int to void
>
> Please find the patch with above changes -
>
>
> From: Narendra K <narendra_k@dell.com>
> Subject: [PATCH] Export SMBIOS provided firmware instance and label to sysfs
>
> This patch exports SMBIOS provided firmware instance and label
> of onboard pci devices to sysfs
>
> Signed-off-by: Jordan Hargrave <jordan_hargrave@dell.com>
> Signed-off-by: Narendra K <narendra_k@dell.com>
> ---
> Documentation/ABI/testing/sysfs-bus-pci | 26 ++++++
> drivers/firmware/dmi_scan.c | 25 ++++++
> drivers/pci/Makefile | 3 +
> drivers/pci/pci-label.c | 143 +++++++++++++++++++++++++++++++
> drivers/pci/pci-sysfs.c | 5 +
> drivers/pci/pci.h | 9 ++
> include/linux/dmi.h | 9 ++
> 7 files changed, 220 insertions(+), 0 deletions(-)
> create mode 100644 drivers/pci/pci-label.c
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> index 428676c..d3eb807 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci
> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> @@ -179,3 +179,29 @@ Contact: linux-pci@vger.kernel.org
> Description:
> This symbolic link points to the PCI hotplug controller driver
> module that manages the hotplug slot.
> +
> +What: /sys/bus/pci/devices/.../label
> +Date: July 2010
> +Contact: Linux PCI developers <linux-pci@vger.kernel.org>
Why not contact you?
> +Description:
> + Reading this attribute will provide the firmware
> + given name of the PCI device. The attribute will
> + be created only if the firmware has given a name
> + to the PCI device.
Where does that "name" come from? Please describe this.
> +Users:
> + Userspace applications interested in knowing the
> + firmware assigned name of the PCI device.
> +
> +What: /sys/bus/pci/devices/.../index
> +Date: July 2010
> +Contact: Linux PCI developers <linux-pci@vger.kernel.org>
> +Description:
> + Reading this attribute will provide the firmware
> + given instance of the PCI device. The attribute will
> + be created only if the firmware has given a device
> + type instance to the PCI device.
Same comments as above.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] Driver-core: Fix bluetooth network device rename regression
From: Greg KH @ 2010-07-22 18:54 UTC (permalink / raw)
To: Johannes Berg
Cc: Kay Sievers, Eric W. Biederman, Andrew Morton, Greg KH,
Rafael J. Wysocki, Maciej W. Rozycki, netdev
In-Reply-To: <1279823801.12439.31.camel@jlt3.sipsolutions.net>
On Thu, Jul 22, 2010 at 08:36:41PM +0200, Johannes Berg wrote:
> On Thu, 2010-07-22 at 11:28 -0700, Greg KH wrote:
>
> > It worked only because no one realized that it was broken with the
> > DEPRECATED option enabled. When that is enabled, it is broken, right?
>
> I'm pretty sure I always had that enabled, and never had issues. Can't
> test right now since I don't have that option back yet in the tree I'm
> using.
>
> > Eric's changes to sysfs to add namespace support exposed this breakage.
> > That's not a reason to paper over the problem, but it should be driving
> > someone to fix it correctly, as has been pointed out a number of times
> > already.
>
> I'm just contesting that that someone should be me. I don't think you
> get to blame driver developers for doing something that worked and
> solved the problem they needed to solve. sysfs is largely opaque to most
> of us already, and it now sure feels like Kay decided to change the
> rules underneath the code in saying "this was wrong all along".
Well, if it worked before, and it doesn't now, that's due to Eric's
changes, nothing Kay and I did here :)
But, in looking at it closer, it does seem that the code is doing things
that was not expected to work at all previously, and It's amazing that
it did. I thought Kay offered to help fix it all up, and provided 2
different ways to do it. I know they aren't trivial, but then again,
your usage of sysfs is not trivial either...
thanks,
greg k-h
^ 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