* [PATCH net v3] ipv6: mld: fix add_grhead skb_over_panic for devs with large MTUs
From: Daniel Borkmann @ 2014-11-05 14:42 UTC (permalink / raw)
To: davem; +Cc: lw1a2.jing, fw, hannes, netdev, Eric Dumazet, David L Stevens
It has been reported that generating an MLD listener report on
devices with large MTUs (e.g. 9000) and a high number of IPv6
addresses can trigger a skb_over_panic():
skbuff: skb_over_panic: text:ffffffff80612a5d len:3776 put:20
head:ffff88046d751000 data:ffff88046d751010 tail:0xed0 end:0xec0
dev:port1
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:100!
invalid opcode: 0000 [#1] SMP
Modules linked in: ixgbe(O)
CPU: 3 PID: 0 Comm: swapper/3 Tainted: G O 3.14.23+ #4
[...]
Call Trace:
<IRQ>
[<ffffffff80578226>] ? skb_put+0x3a/0x3b
[<ffffffff80612a5d>] ? add_grhead+0x45/0x8e
[<ffffffff80612e3a>] ? add_grec+0x394/0x3d4
[<ffffffff80613222>] ? mld_ifc_timer_expire+0x195/0x20d
[<ffffffff8061308d>] ? mld_dad_timer_expire+0x45/0x45
[<ffffffff80255b5d>] ? call_timer_fn.isra.29+0x12/0x68
[<ffffffff80255d16>] ? run_timer_softirq+0x163/0x182
[<ffffffff80250e6f>] ? __do_softirq+0xe0/0x21d
[<ffffffff8025112b>] ? irq_exit+0x4e/0xd3
[<ffffffff802214bb>] ? smp_apic_timer_interrupt+0x3b/0x46
[<ffffffff8063f10a>] ? apic_timer_interrupt+0x6a/0x70
mld_newpack() skb allocations are usually requested with dev->mtu
in size, since commit 72e09ad107e7 ("ipv6: avoid high order allocations")
we have changed the limit in order to be less likely to fail.
However, in MLD/IGMP code, we have some rather ugly AVAILABLE(skb)
macros, which determine if we may end up doing an skb_put() for
adding another record. To avoid possible fragmentation, we check
the skb's tailroom as skb->dev->mtu - skb->len, which is a wrong
assumption as the actual max allocation size can be much smaller.
The IGMP case doesn't have this issue as commit 57e1ab6eaddc
("igmp: refine skb allocations") stores the allocation size in
the cb[].
Set a reserved_tailroom to make it fit into the MTU and use
skb_availroom() helper instead. This also allows to get rid of
igmp_skb_size().
Reported-by: Wei Liu <lw1a2.jing@gmail.com>
Fixes: 72e09ad107e7 ("ipv6: avoid high order allocations")
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: David L Stevens <david.stevens@oracle.com>
---
v2->v3:
- Still had a discussion w/ Hannes and improved the code a bit to
make it more clear to read
v1->v2:
- Don't introduce skb_nofrag_tailroom(), but reuse skb_availroom()
as suggested by Eric
net/ipv4/igmp.c | 17 +++++++----------
net/ipv6/mcast.c | 19 ++++++++++---------
2 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index fb70e3e..d90bdbf 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -318,9 +318,7 @@ igmp_scount(struct ip_mc_list *pmc, int type, int gdeleted, int sdeleted)
return scount;
}
-#define igmp_skb_size(skb) (*(unsigned int *)((skb)->cb))
-
-static struct sk_buff *igmpv3_newpack(struct net_device *dev, int size)
+static struct sk_buff *igmpv3_newpack(struct net_device *dev, unsigned int mtu)
{
struct sk_buff *skb;
struct rtable *rt;
@@ -330,6 +328,7 @@ static struct sk_buff *igmpv3_newpack(struct net_device *dev, int size)
struct flowi4 fl4;
int hlen = LL_RESERVED_SPACE(dev);
int tlen = dev->needed_tailroom;
+ unsigned int size = mtu;
while (1) {
skb = alloc_skb(size + hlen + tlen,
@@ -340,20 +339,19 @@ static struct sk_buff *igmpv3_newpack(struct net_device *dev, int size)
if (size < 256)
return NULL;
}
- skb->priority = TC_PRIO_CONTROL;
- igmp_skb_size(skb) = size;
rt = ip_route_output_ports(net, &fl4, NULL, IGMPV3_ALL_MCR, 0,
- 0, 0,
- IPPROTO_IGMP, 0, dev->ifindex);
+ 0, 0, IPPROTO_IGMP, 0, dev->ifindex);
if (IS_ERR(rt)) {
kfree_skb(skb);
return NULL;
}
+ skb->priority = TC_PRIO_CONTROL;
skb_dst_set(skb, &rt->dst);
skb->dev = dev;
-
+ skb->reserved_tailroom = skb_end_offset(skb) -
+ min(mtu, skb_end_offset(skb));
skb_reserve(skb, hlen);
skb_reset_network_header(skb);
@@ -423,8 +421,7 @@ static struct sk_buff *add_grhead(struct sk_buff *skb, struct ip_mc_list *pmc,
return skb;
}
-#define AVAILABLE(skb) ((skb) ? ((skb)->dev ? igmp_skb_size(skb) - (skb)->len : \
- skb_tailroom(skb)) : 0)
+#define AVAILABLE(skb) ((skb) ? skb_availroom(skb) : 0)
static struct sk_buff *add_grec(struct sk_buff *skb, struct ip_mc_list *pmc,
int type, int gdeleted, int sdeleted)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 9648de2..d817737 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1550,7 +1550,7 @@ static void ip6_mc_hdr(struct sock *sk, struct sk_buff *skb,
hdr->daddr = *daddr;
}
-static struct sk_buff *mld_newpack(struct inet6_dev *idev, int size)
+static struct sk_buff *mld_newpack(struct inet6_dev *idev, unsigned int mtu)
{
struct net_device *dev = idev->dev;
struct net *net = dev_net(dev);
@@ -1560,22 +1560,23 @@ static struct sk_buff *mld_newpack(struct inet6_dev *idev, int size)
struct in6_addr addr_buf;
const struct in6_addr *saddr;
int hlen = LL_RESERVED_SPACE(dev);
- int tlen = dev->needed_tailroom;
- int err;
+ int err, tlen = dev->needed_tailroom;
+ unsigned int size = mtu + hlen + tlen;
u8 ra[8] = { IPPROTO_ICMPV6, 0,
IPV6_TLV_ROUTERALERT, 2, 0, 0,
IPV6_TLV_PADN, 0 };
- /* we assume size > sizeof(ra) here */
- size += hlen + tlen;
- /* limit our allocations to order-0 page */
+ /* We assume size > sizeof(ra) here. Limit our
+ * allocations to order-0 page.
+ */
size = min_t(int, size, SKB_MAX_ORDER(0, 0));
skb = sock_alloc_send_skb(sk, size, 1, &err);
-
if (!skb)
return NULL;
skb->priority = TC_PRIO_CONTROL;
+ skb->reserved_tailroom = skb_end_offset(skb) -
+ min(mtu, skb_end_offset(skb));
skb_reserve(skb, hlen);
if (__ipv6_get_lladdr(idev, &addr_buf, IFA_F_TENTATIVE)) {
@@ -1599,6 +1600,7 @@ static struct sk_buff *mld_newpack(struct inet6_dev *idev, int size)
pmr->mld2r_cksum = 0;
pmr->mld2r_resv2 = 0;
pmr->mld2r_ngrec = 0;
+
return skb;
}
@@ -1690,8 +1692,7 @@ static struct sk_buff *add_grhead(struct sk_buff *skb, struct ifmcaddr6 *pmc,
return skb;
}
-#define AVAILABLE(skb) ((skb) ? ((skb)->dev ? (skb)->dev->mtu - (skb)->len : \
- skb_tailroom(skb)) : 0)
+#define AVAILABLE(skb) ((skb) ? skb_availroom(skb) : 0)
static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
int type, int gdeleted, int sdeleted, int crsend)
--
1.7.11.7
^ permalink raw reply related
* [PATCH] net/9p: remove a comment about pref member which doesn't exist
From: Ryo Munakata @ 2014-11-05 14:45 UTC (permalink / raw)
To: ericvh; +Cc: rminnich, lucho, davem, netdev, linux-kernel, Ryo Munakata
Signed-off-by: Ryo Munakata <ryomnktml@gmail.com>
---
include/net/9p/transport.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/net/9p/transport.h b/include/net/9p/transport.h
index d9fa68f..2a25dec 100644
--- a/include/net/9p/transport.h
+++ b/include/net/9p/transport.h
@@ -34,7 +34,6 @@
* @list: used to maintain a list of currently available transports
* @name: the human-readable name of the transport
* @maxsize: transport provided maximum packet size
- * @pref: Preferences of this transport
* @def: set if this transport should be considered the default
* @create: member function to create a new connection on this transport
* @close: member function to discard a connection on this transport
--
2.1.3
^ permalink raw reply related
* Re: [PATCH] net: mv643xx_eth: reclaim TX skbs only when released by the HW
From: Ezequiel Garcia @ 2014-11-05 14:46 UTC (permalink / raw)
To: Karl Beldan, David Miller, Ian Campbell
Cc: Karl Beldan, netdev, Eric Dumazet, Sebastian Hesselbarth
In-Reply-To: <1415197979-1702-1-git-send-email-karl.beldan@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]
Hi Karl,
On 11/05/2014 11:32 AM, Karl Beldan wrote:> From: Karl Beldan <karl.beldan@rivierawaves.com>
>
> ATM, txq_reclaim will dequeue and free an skb for each tx desc released
> by the hw that has TX_LAST_DESC set. However, in case of TSO, each
> hw desc embedding the last part of a segment has TX_LAST_DESC set,
> losing the one-to-one 'last skb frag'/'TX_LAST_DESC set' correspondance,
> which causes data corruption.
>
> Fix this by checking TX_ENABLE_INTERRUPT instead of TX_LAST_DESC, and
> warn when trying to dequeue from an empty txq (which can be symptomatic
> of releasing skbs prematurely).
>
> Fixes: 3ae8f4e0b98 ('net: mv643xx_eth: Implement software TSO')
Although your change makes sense, this isn't fixing the issue for me,
neither did the previous one.
Ian: Can you double check that you have corruption *without* the patch,
and that the patch fixes the issue?
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 1/7] bpf: add 'flags' attribute to BPF_MAP_UPDATE_ELEM command
From: Daniel Borkmann @ 2014-11-05 14:57 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Andy Lutomirski,
Hannes Frederic Sowa, Eric Dumazet, Linux API,
Network Development, LKML
In-Reply-To: <CAMEtUuy5gJbeLqiSr1=SiNQ7WyqocUVV-siwhEnpBVqmzYzzCQ@mail.gmail.com>
On 11/05/2014 12:04 AM, Alexei Starovoitov wrote:
> On Tue, Nov 4, 2014 at 1:25 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
>> On 11/04/2014 03:54 AM, Alexei Starovoitov wrote:
>>>
>>> the current meaning of BPF_MAP_UPDATE_ELEM syscall command is:
>>> either update existing map element or create a new one.
>>> Initially the plan was to add a new command to handle the case of
>>> 'create new element if it didn't exist', but 'flags' style looks
>>> cleaner and overall diff is much smaller (more code reused), so add 'flags'
>>> attribute to BPF_MAP_UPDATE_ELEM command with the following meaning:
>>> enum {
>>> BPF_MAP_UPDATE_OR_CREATE = 0, /* add new element or update existing */
>>> BPF_MAP_CREATE_ONLY, /* add new element if it didn't exist */
>>> BPF_MAP_UPDATE_ONLY /* update existing element */
>>> };
>>
>> From you commit message/code I currently don't see an explanation why
>> it cannot be done in typical ``flags style'' as various syscalls do,
>> i.e. BPF_MAP_UPDATE_OR_CREATE rather represented as ...
>>
>> BPF_MAP_CREATE | BPF_MAP_UPDATE
>>
>> Do you expect more than 64 different flags to be passed from user space
>> for BPF_MAP?
>
> several reasons:
> - preserve flags==0 as default behavior
> - avoid holes and extra checks for invalid combinations, so
> if (flags > BPF_MAP_UPDATE_ONLY) goto err, is enough.
> - it looks much neater when user space uses
> BPF_MAP_UPDATE_OR_CREATE instead of ORing bits.
>
> Note this choice doesn't prevent adding bit-like flags
> in the future. Today I cannot think of any new flags
> for the update() command, but if somebody comes up with
> a new selector that can apply to all three combinations,
> we can add it as 3rd bit that can be ORed.
Hm, mixing enums together with bitfield-like flags seems
kind of hacky ... :/ Or, do you mean to say you're adding
a 2nd flag field, i.e. splitting the 64bits into a 32bit
``cmd enum'' and 32bit ``flag section''?
I see the point with flags == 0 as default behavior though,
but at this point in time we won't get burnt by it since
the API is not yet in a usable state and defaults to be
compiled-out.
> Default will stay zero and 'if >' check in older
> kernels will seamlessly work with new userspace.
> I don't like holes in flags and combinatorial
> explosion of bits and checks for them unless
> absolutely necessary.
Hm, my concern is that we start to add many *_OR_* enum
elements once we find that a flag might be a useful in
combination with many other flags ... even though if we
only can think of 3 flags /right now/.
^ permalink raw reply
* Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net onarndale platform
From: Charles Keepax @ 2014-11-05 15:02 UTC (permalink / raw)
To: Stam, Michel [FINT]
Cc: Riku Voipio, davem, linux-usb, netdev, linux-kernel,
linux-samsung-soc
In-Reply-To: <C89EFD3CD56F64468D3D206D683A8D22039FFC5F@ldam-msx2.fugro-nl.local>
On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote:
> Hello Charles,
>
> After looking around I found the reset value for the 8772 chip, which
> seems to be 0x1E1 (ANAR register).
>
> This equates to (according to include/uapi/linux/mii.h)
> ADVERTISE_ALL | ADVERTISE_CSMA.
>
> The register only seems to become 0 if the software reset fails.
Odd it definitely reads back as zero on Arndale. I am guessing
that the root of the problem here is that for some reason Arndale
POR of the ethernet is pants and it needs a full software reset
before it will work and the patch removes the full reset
callback.
>
> Unfortunately, this is exactly what I get when the patch is applied;
> asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5
> asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2,
> ASIX AX88772 USB 2.0 Ethernet
> asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5
> asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2,
> ASIX AX88772 USB 2.0 Ethernet
Ok so I am guessing you have a value in the register which is
neither the reset value or 0 and this causing problems later in
the reset/on the next reset. I do find the naming confusing in
the error message there as it says link reset failed but the
link_reset callback can't fail in the driver and I modified the
reset callback. But I guess that is just oddities of the network
stack I am not familiar with.
The other thing that feels odd is (and again apologies as I know
next to nothing about the networking stack) how come it is
unexpected that the reset callback destroys the state of the
device. Naively I would have expected that a reset callback would
reset the device back to its default state. Here we seem to be
trying to avoid that happening.
>
> A little while after this its 'Failed to enable software MII access'.
> The ethernet device fails to see any link or accept any ethtool -s
> command.
>
> My device has vid:devid 0b95:772a (ASIX Elec. Corp.).
>
> Can you tell me what device is on the Andale platform, Charles? Same
> vendor/device id?
Yeah mine also idVendor=0b95, idProduct=772a
Thanks,
Charles
>
> Another thing that bothers me is that dev->mii.advertising seems to
> contain the same value, so maybe that can be used instead of a call to
> asix_mdio_read(). Can anyone comment on its purpose? Should it be a
> shadow copy of the real register or something?
>
> Riku, can you test Charles' patch as well?
>
> Kind regards,
>
> Michel Stam
>
> -----Original Message-----
> From: Charles Keepax [mailto:ckeepax@opensource.wolfsonmicro.com]
> Sent: Tuesday, November 04, 2014 21:09 PM
> To: Stam, Michel [FINT]
> Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-samsung-soc@vger.kernel.org
> Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net
> onarndale platform
>
> On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote:
> > Hello Riku,
> >
> > >Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> > this case on arndale). This is now a regression of functionality from
> > 3.17.
> > >
> > >I think it would better to revert the change now and with less hurry
> > introduce a ethtool fix that doesn't break arndale.
> >
> > I don't fully agree here;
> > I would like to point out that this commit is a revert itself. Fixing
> > the armdale will then cause breakage in other implementations, such as
>
> > ours. Blankly reverting breaks other peoples' implementations.
> >
> > The PHY reset is the thing that breaks ethtool support, so any fix
> > that appeases all would have to take existing PHY state into account.
> >
> > I'm not an expert on the ASIX driver, nor the MII, but I think this is
>
> > the cause;
> > drivers/net/usb/asix_devices.c:
> > 361 asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> > BMCR_RESET);
> > 362 asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> > 363 ADVERTISE_ALL | ADVERTISE_CSMA);
> > 364 mii_nway_restart(&dev->mii);
> >
> > I would think that the ADVERTISE_ALL is the cause here, as it will
> > reset the MII back to default, thus overriding ethtool settings.
> > Would an:
> > Int reg;
> > reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
> >
> > prior to the offending lines, and then;
> >
> > 362 asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> > 363 reg);
> >
> > solve the problem for you guys?
>
> If I revert the patch in question and add this in:
>
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev) {
> struct asix_data *data = (struct asix_data *)&dev->data;
> int ret, embd_phy;
> + int reg;
> u16 rx_ctl;
>
> ret = asix_write_gpio(dev,
> @@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
> msleep(150);
>
> asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> BMCR_RESET);
> - asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> - ADVERTISE_ALL | ADVERTISE_CSMA);
> + reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_ADVERTISE);
> + if (!reg)
> + reg = ADVERTISE_ALL | ADVERTISE_CSMA;
> + asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, reg);
> mii_nway_restart(&dev->mii);
>
> ret = asix_write_medium_mode(dev, AX88772_MEDIUM_DEFAULT);
>
> Then things work on Arndale for me. Does that work for you?
> Whether that is a sensible fix I don't know however.
>
> >
> > Mind, maybe the read function should take into account the reset value
>
> > of the MII, and set it to ADVERTISE_ALL | ADVERTISE_CSMA. I don't have
>
> > any documention here at the moment.
>
> Yeah I also have no documentation.
>
> Thanks,
> Charles
>
> >
> > Is anyone able to confirm my suspicions?
> >
> > Kind regards,
> >
> > Michel Stam
> > -----Original Message-----
> > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > Sent: Tuesday, November 04, 2014 10:44 AM
> > To: Stam, Michel [FINT]
> > Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org;
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux-samsung-soc@vger.kernel.org; ckeepax@opensource.wolfsonmicro.com
> > Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks
> > net on arndale platform
> >
> > On Tue, Nov 04, 2014 at 09:19:26AM +0100, Stam, Michel [FINT] wrote:
> > > Interesting, as the commit itself is a revert from a kernel back to
> > > 2.6 somewhere. The problem I had is related to the PHY being reset
> > > on interface-up, can you confirm that you require this?
> >
> > I can't confirm what exactly is needed on arndale. I'm neither expert
> > in USB or ethernet. However, I can confirm that without the PHY reset,
>
> > networking doesn't work on arndale.
> >
> > I now see someone else has the same problem, adding Charles to CC.
> >
> > http://www.spinics.net/lists/linux-usb/msg116656.html
> >
> > > Reverting this
> > > breaks ethtool support in turn.
> >
> > Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> > this case on arndale). This is now a regression of functionality from
> > 3.17.
> >
> > I think it would better to revert the change now and with less hurry
> > introduce a ethtool fix that doesn't break arndale.
> >
> > > Kind regards,
> > >
> > > Michel Stam
> > >
> > > -----Original Message-----
> > > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > > Sent: Tuesday, November 04, 2014 8:23 AM
> > > To: davem@davemloft.net; Stam, Michel [FINT]
> > > Cc: linux-usb@vger.kernel.org; netdev@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> > > Subject: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net
> > > on
> >
> > > arndale platform
> > >
> > > Hi,
> > >
> > > With 3.18-rc3, asix on arndale (samsung exynos 5250 based board),
> > > fails to work. Interface is initialized but network traffic seem not
>
> > > to pass through. With kernel IP config the result looks like:
> > >
> > > [ 3.323275] usb 3-3.2.4: new high-speed USB device number 4 using
> > > exynos-ehci
> > > [ 3.419151] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [ 3.424735] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [ 3.432196] usb 3-3.2.4: Product: AX88772
> > > [ 3.436279] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [ 3.441486] usb 3-3.2.4: SerialNumber: 000001
> > > [ 3.447530] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [ 3.764352] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > de:a2:66:bf:ca:4f
> > > [ 4.488773] asix 3-3.2.4:1.0 eth0: link down
> > > [ 5.690025] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> > lpa
> > > 0xC5E1
> > > [ 5.712947] Sending DHCP requests ...... timed out!
> > > [ 83.165303] IP-Config: Retrying forever (NFS root)...
> > > [ 83.170397] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> > lpa
> > > 0xC5E1
> > > [ 83.192944] Sending DHCP requests .....
> > >
> > > Similar results also with dhclient. Git bisect identified the
> > > breaking
> >
> > > commit as:
> > >
> > > commit 3cc81d85ee01e5a0b7ea2f4190e2ed1165f53c31
> > > Author: Michel Stam <m.stam@fugro.nl>
> > > Date: Thu Oct 2 10:22:02 2014 +0200
> > >
> > > asix: Don't reset PHY on if_up for ASIX 88772
> > >
> > > Taking 3.18-rc3 and that commit reverted, network works again:
> > >
> > > [ 3.303500] usb 3-3.2.4: new high-speed USB device number 4 using
> > > exynos-ehci
> > > [ 3.399375] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [ 3.404963] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [ 3.412424] usb 3-3.2.4: Product: AX88772
> > > [ 3.416508] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [ 3.421715] usb 3-3.2.4: SerialNumber: 000001
> > > [ 3.427755] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [ 3.744837] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > 12:59:f1:a8:43:90
> > > [ 7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> > lpa
> > > 0xC5E1
> > > [ 7.118258] Sending DHCP requests ., OK
> > > [ 9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my
> address
> > > is 192.168.1.111
> > >
> > > There might something wrong on the samsung platform code (I
> > > understand
> >
> > > the USB on arndale is "funny"), but this is still an regression from
>
> > > 3.17.
> > >
> > > Riku
^ permalink raw reply
* mlx4+vxlan offload breaks gre tunnels
From: Florian Westphal @ 2014-11-05 15:04 UTC (permalink / raw)
To: netdev; +Cc: amirv, ogerlitz
tl,dr: all tcp packets sent via gre tunnel have broken tcp csum if vxlan offload
is enabled with mlx4 driver.
Given following config on tx-side:
dev=enp3s0
ip addr add dev $dev 192.168.23.1/24
ip link set $dev up
ip link add mygre type gretap remote 192.168.23.2 local 192.168.23.1
ip addr add dev mygre 192.168.42.1/24
ip link set gre0 up
ip link set mygre up
and
options mlx4_core log_num_mgm_entry_size=-1 debug_level=1
port_type_array=2,2
in
/etc/modprobe.d/mlx4.conf
all tcp packets sent to destinations over the gre tunnel have bogus tcp
checksums (and are tossed on rx side when stack validates tcp checksum).
net-next head is commit 30349bdbc4da5ecf0efa25556e3caff9c9b8c5f7 .
What makes things work for me:
either
options mlx4_core 1 debug_level=1 port_type_array=2,2
(ie. no MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
or not setting NETIF_F_IP_CSUM in enc_features:
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2579,10 +2579,12 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
dev->priv_flags |= IFF_UNICAST_FLT;
if (mdev->dev->caps.tunnel_offload_mode == MLX4_TUNNEL_OFFLOAD_MODE_VXLAN) {
- dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
+ dev->hw_enc_features |= NETIF_F_RXCSUM |
NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL;
I am not sure if its right fix, but to my eyes this basically looks like
mlx4 is telling stack that it can handle tcp checksum offload within
tunnels, and that doesn't seem to be the case for all types (e.g. gre).
Could someone who understand the enc_features specifics better confirm that
above patch is correct (or provide a better/proper fix)?
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH] net: mv643xx_eth: reclaim TX skbs only when released by the HW
From: Karl Beldan @ 2014-11-05 15:05 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: David Miller, Ian Campbell, Karl Beldan, netdev, Eric Dumazet,
Sebastian Hesselbarth
In-Reply-To: <545A3838.3090606@free-electrons.com>
On Wed, Nov 05, 2014 at 11:46:16AM -0300, Ezequiel Garcia wrote:
> Hi Karl,
>
> On 11/05/2014 11:32 AM, Karl Beldan wrote:> From: Karl Beldan <karl.beldan@rivierawaves.com>
> >
> > ATM, txq_reclaim will dequeue and free an skb for each tx desc released
> > by the hw that has TX_LAST_DESC set. However, in case of TSO, each
> > hw desc embedding the last part of a segment has TX_LAST_DESC set,
> > losing the one-to-one 'last skb frag'/'TX_LAST_DESC set' correspondance,
> > which causes data corruption.
> >
> > Fix this by checking TX_ENABLE_INTERRUPT instead of TX_LAST_DESC, and
> > warn when trying to dequeue from an empty txq (which can be symptomatic
> > of releasing skbs prematurely).
> >
> > Fixes: 3ae8f4e0b98 ('net: mv643xx_eth: Implement software TSO')
>
> Although your change makes sense, this isn't fixing the issue for me,
> neither did the previous one.
>
This change fixes a serious issue.
On my side I can now trigger misc NFS and md5sums errors very easily,
which I haven't detected so far with it applied.
Are you running little endian ? Do you have the tso alignment fix
a63ba13e (I don't expect it to be required but I don't know what SoC you
are using) ? I suppose you are running with all 3 fixes applied.
Karl
^ permalink raw reply
* Re: [PATCH 02/20] selftests/net: move test out of Makefile into a shell script
From: Shuah Khan @ 2014-11-05 15:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20141104.145315.548353349719796428.davem@davemloft.net>
On 11/04/2014 12:53 PM, David Miller wrote:
> From: Shuah Khan <shuahkh@osg.samsung.com>
> Date: Tue, 4 Nov 2014 10:10:58 -0700
>
>> Currently bpf test run from the Makefile. Move it out of the
>> Makefile to be run from a shell script to allow the test to
>> be run as stand-alone test, in addition to allowing the test
>> run from a make target.
>>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>
> Acked-by: David S. Miller <davem@davemloft.net>
>
Thanks. Queuing this patch up for 3.19-rc1.
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
^ permalink raw reply
* Re: [PATCH v2 net] tcp: zero retrans_stamp if all retrans were acked
From: Marcelo Ricardo Leitner @ 2014-11-05 15:32 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Neal Cardwell, Netdev, Yuchung Cheng, Eric Dumazet
In-Reply-To: <1415150445.1458.1.camel@edumazet-glaptop2.roam.corp.google.com>
On 04-11-2014 23:20, Eric Dumazet wrote:
> On Tue, 2014-11-04 at 18:51 -0200, Marcelo Ricardo Leitner wrote:
>
>> And thank you guys for all the assistance on it. Btw, would you send me that
>> packetdrill script? I'm curious to see how such corner case could be written
>> on it.
>
> One of the script I saw was :
>
> You might have to adapt preconditions (tcp_rmem[]/tcp_wmem[])
>
> 0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
> // Set a 10s timeout
> +.000 setsockopt(3, SOL_TCP, TCP_USER_TIMEOUT, [10000], 4) = 0
> +.000 bind(3, ..., ...) = 0
> +.000 listen(3, 1) = 0
> +.000 < S 0:0(0) win 32792 <mss 1460,nop,wscale 7>
> +.000 > S. 0:0(0) ack 1 <mss 1460,nop,wscale 6>
> +.010 < . 1:1(0) ack 1 win 257
> +.000 accept(3, ..., ...) = 4
> +.000 write(4, ..., 1000) = 1000
> +.000 > P. 1:1001(1000) ack 1
> +.625 > P. 1:1001(1000) ack 1
> +.020 < . 1:1(0) ack 1001 win 257
> // Purposely write more after the specified timeout for testing
> +11.0 write(4, ..., 1000) = 1000
> +.000 > P. 1001:2001(1000) ack 1
> +1.25 > P. 1001:2001(1000) ack 1
> // socket is killed when the 2nd RTO fires at +2.50 w/o this patch
> // so the next write returns ETIMEOUT
> +2.60 write(4, ..., 1000) = 1000
Cool, thanks!
Marcelo
^ permalink raw reply
* Re: [PATCH] net: mv643xx_eth: reclaim TX skbs only when released by the HW
From: Eric Dumazet @ 2014-11-05 15:41 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Karl Beldan, David Miller, Ian Campbell, Karl Beldan, netdev,
Sebastian Hesselbarth
In-Reply-To: <545A3838.3090606@free-electrons.com>
On Wed, 2014-11-05 at 11:46 -0300, Ezequiel Garcia wrote:
> Hi Karl,
>
> On 11/05/2014 11:32 AM, Karl Beldan wrote:> From: Karl Beldan <karl.beldan@rivierawaves.com>
> >
> > ATM, txq_reclaim will dequeue and free an skb for each tx desc released
> > by the hw that has TX_LAST_DESC set. However, in case of TSO, each
> > hw desc embedding the last part of a segment has TX_LAST_DESC set,
> > losing the one-to-one 'last skb frag'/'TX_LAST_DESC set' correspondance,
> > which causes data corruption.
> >
> > Fix this by checking TX_ENABLE_INTERRUPT instead of TX_LAST_DESC, and
> > warn when trying to dequeue from an empty txq (which can be symptomatic
> > of releasing skbs prematurely).
> >
> > Fixes: 3ae8f4e0b98 ('net: mv643xx_eth: Implement software TSO')
>
> Although your change makes sense, this isn't fixing the issue for me,
> neither did the previous one.
>
> Ian: Can you double check that you have corruption *without* the patch,
> and that the patch fixes the issue?
>
Have you also applied my patch ?
^ permalink raw reply
* Re: Possible bug in net/core/neighbor.c
From: Ulf Samuelsson @ 2014-11-05 15:46 UTC (permalink / raw)
To: netdev
In-Reply-To: <5459FFEA.3000101@ericsson.com>
On 11/05/2014 11:46 AM, Ulf Samuelsson wrote:
> I find the following in "net/core/neighbor.c"
>
> /* Compare new lladdr with cached one */
> if (!dev->addr_len) {
> /* First case: device needs no address. */
> lladdr = neigh->ha;
> } else if (lladdr) {
> /* The second case: if something is already cached
> and a new address is proposed:
> - compare new & old
> - if they are different, check override flag
> */
>
> /* POSSIBLE BUG */
> if ((old & NUD_VALID) &&
> !memcmp(lladdr, neigh->ha, dev->addr_len))
> lladdr = neigh->ha;
> /* END POSSIBLE BUG */
> } else {
> /* No address is supplied; if we know something,
> use it, otherwise discard the request.
> */
> err = -EINVAL;
> if (!(old & NUD_VALID))
> goto out;
> lladdr = neigh->ha;
> }
>
> It looks to me like the code is saying
> if the proposed address is equal to the original address,
> set the proposed address to the original address.
>
> which is pretty meaningless.
>
> Should it not be:
>
> if ((old & NUD_VALID) &&
> memcmp(lladdr, neigh->ha, dev->addr_len)) /* True if
> addresses are not equal */
> neigh->ha = lladdr; /* Update to new address */
>
> If not, I would appreciate an explanation what the code is doing.
>
OK, I think I figured it out.
If laddr and neigh->ha are identical, we want lladdr (which is a pointer)
to have the same value as neigh->ha so after this, you know that
laddr is identical to neigh->ha by justr comparing the pointers.
When I google, I only find other people which does not understand
the purpose of the code.
The comments are also obsolete, since "check override flag" refers
to code which has been removed.
Best Regards,
Ulf Samuelsson
KI/EAB/ILM/GF
ulf.samuelsson@ericsson.com
+46 722 427 437
^ permalink raw reply
* RE: "asix: Don't reset PHY on if_up for ASIX 88772" breaks netonarndale platform
From: Stam, Michel [FINT] @ 2014-11-05 16:17 UTC (permalink / raw)
To: Charles Keepax
Cc: Riku Voipio, davem, linux-usb, netdev, linux-kernel,
linux-samsung-soc
In-Reply-To: <20141105150258.GR23178@opensource.wolfsonmicro.com>
Hello Charles,
First of all, my apologies. I manually applied your patch and made a
mistake; I swapped ax88772_link_reset with ax88772_reset for struct
driver_info_ax88772_info structure, which caused the software reset
failures. Blame it on a lack of coffee... Please disregard my previous
mail.
After (correctly) applying the patch I still don't get the desired
results; see below.
Test situation:
ifconfig eth1 down
ethtool -s advertise 1
ifconfig eth1 up
Check dmesg, It will report a link down, followed by the new speed,
which when using advertise 1, should be 10 Mbps/half duplex.
To make it more interesting, ethtool eth1 reports 10 Mbps/half duplex
even though the kernel reports 100 Mbps/full duplex.
What does work (but did before this whole situation as well):
ifconfig eth1 up
ethtool -s eth1 advertise 1
The interface will now be in 10 Mbps/half duplex, that is, until the
reset function is called (interface down or otherwise).
What I do notice, is that dev->mii.advertising follows whatever I set
with ethtool. I tried writing the ADVERTISE register with the value of
the dev->mii.advertising variable, but that did not give me the desired
result either.
Kind regards,
Michel Stam
-----Original Message-----
From: Charles Keepax [mailto:ckeepax@opensource.wolfsonmicro.com]
Sent: Wednesday, November 05, 2014 16:03 PM
To: Stam, Michel [FINT]
Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org;
netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
linux-samsung-soc@vger.kernel.org
Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks
netonarndale platform
On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote:
> Hello Charles,
>
> After looking around I found the reset value for the 8772 chip, which
> seems to be 0x1E1 (ANAR register).
>
> This equates to (according to include/uapi/linux/mii.h) ADVERTISE_ALL
> | ADVERTISE_CSMA.
>
> The register only seems to become 0 if the software reset fails.
Odd it definitely reads back as zero on Arndale. I am guessing that the
root of the problem here is that for some reason Arndale POR of the
ethernet is pants and it needs a full software reset before it will work
and the patch removes the full reset callback.
>
> Unfortunately, this is exactly what I get when the patch is applied;
> asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5 asix
> 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2, ASIX
> AX88772 USB 2.0 Ethernet asix 1-2:1.0 eth1: Failed to send software
> reset: ffffffb5 asix 1-2:1.0 eth1: link reset failed (-75) usbnet
> usb-0000:00:1d.0-2, ASIX AX88772 USB 2.0 Ethernet
Ok so I am guessing you have a value in the register which is neither
the reset value or 0 and this causing problems later in the reset/on the
next reset. I do find the naming confusing in the error message there as
it says link reset failed but the link_reset callback can't fail in the
driver and I modified the reset callback. But I guess that is just
oddities of the network stack I am not familiar with.
The other thing that feels odd is (and again apologies as I know next to
nothing about the networking stack) how come it is unexpected that the
reset callback destroys the state of the device. Naively I would have
expected that a reset callback would reset the device back to its
default state. Here we seem to be trying to avoid that happening.
>
> A little while after this its 'Failed to enable software MII access'.
> The ethernet device fails to see any link or accept any ethtool -s
> command.
>
> My device has vid:devid 0b95:772a (ASIX Elec. Corp.).
>
> Can you tell me what device is on the Andale platform, Charles? Same
> vendor/device id?
Yeah mine also idVendor=0b95, idProduct=772a
Thanks,
Charles
>
> Another thing that bothers me is that dev->mii.advertising seems to
> contain the same value, so maybe that can be used instead of a call to
> asix_mdio_read(). Can anyone comment on its purpose? Should it be a
> shadow copy of the real register or something?
>
> Riku, can you test Charles' patch as well?
>
> Kind regards,
>
> Michel Stam
>
> -----Original Message-----
> From: Charles Keepax [mailto:ckeepax@opensource.wolfsonmicro.com]
> Sent: Tuesday, November 04, 2014 21:09 PM
> To: Stam, Michel [FINT]
> Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-samsung-soc@vger.kernel.org
> Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks
> net onarndale platform
>
> On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote:
> > Hello Riku,
> >
> > >Fixing a bug (ethtool support) must not cause breakage elsewhere
> > >(in
> > this case on arndale). This is now a regression of functionality
> > from 3.17.
> > >
> > >I think it would better to revert the change now and with less
> > >hurry
> > introduce a ethtool fix that doesn't break arndale.
> >
> > I don't fully agree here;
> > I would like to point out that this commit is a revert itself.
> > Fixing the armdale will then cause breakage in other
> > implementations, such as
>
> > ours. Blankly reverting breaks other peoples' implementations.
> >
> > The PHY reset is the thing that breaks ethtool support, so any fix
> > that appeases all would have to take existing PHY state into
account.
> >
> > I'm not an expert on the ASIX driver, nor the MII, but I think this
> > is
>
> > the cause;
> > drivers/net/usb/asix_devices.c:
> > 361 asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> > BMCR_RESET);
> > 362 asix_mdio_write(dev->net, dev->mii.phy_id,
MII_ADVERTISE,
> > 363 ADVERTISE_ALL | ADVERTISE_CSMA);
> > 364 mii_nway_restart(&dev->mii);
> >
> > I would think that the ADVERTISE_ALL is the cause here, as it will
> > reset the MII back to default, thus overriding ethtool settings.
> > Would an:
> > Int reg;
> > reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
> >
> > prior to the offending lines, and then;
> >
> > 362 asix_mdio_write(dev->net, dev->mii.phy_id,
MII_ADVERTISE,
> > 363 reg);
> >
> > solve the problem for you guys?
>
> If I revert the patch in question and add this in:
>
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev) {
> struct asix_data *data = (struct asix_data *)&dev->data;
> int ret, embd_phy;
> + int reg;
> u16 rx_ctl;
>
> ret = asix_write_gpio(dev,
> @@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
> msleep(150);
>
> asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> BMCR_RESET);
> - asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> - ADVERTISE_ALL | ADVERTISE_CSMA);
> + reg = asix_mdio_read(dev->net, dev->mii.phy_id,
MII_ADVERTISE);
> + if (!reg)
> + reg = ADVERTISE_ALL | ADVERTISE_CSMA;
> + asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> + reg);
> mii_nway_restart(&dev->mii);
>
> ret = asix_write_medium_mode(dev, AX88772_MEDIUM_DEFAULT);
>
> Then things work on Arndale for me. Does that work for you?
> Whether that is a sensible fix I don't know however.
>
> >
> > Mind, maybe the read function should take into account the reset
> > value
>
> > of the MII, and set it to ADVERTISE_ALL | ADVERTISE_CSMA. I don't
> > have
>
> > any documention here at the moment.
>
> Yeah I also have no documentation.
>
> Thanks,
> Charles
>
> >
> > Is anyone able to confirm my suspicions?
> >
> > Kind regards,
> >
> > Michel Stam
> > -----Original Message-----
> > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > Sent: Tuesday, November 04, 2014 10:44 AM
> > To: Stam, Michel [FINT]
> > Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org;
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux-samsung-soc@vger.kernel.org;
> > ckeepax@opensource.wolfsonmicro.com
> > Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks
> > net on arndale platform
> >
> > On Tue, Nov 04, 2014 at 09:19:26AM +0100, Stam, Michel [FINT] wrote:
> > > Interesting, as the commit itself is a revert from a kernel back
> > > to
> > > 2.6 somewhere. The problem I had is related to the PHY being reset
> > > on interface-up, can you confirm that you require this?
> >
> > I can't confirm what exactly is needed on arndale. I'm neither
> > expert in USB or ethernet. However, I can confirm that without the
> > PHY reset,
>
> > networking doesn't work on arndale.
> >
> > I now see someone else has the same problem, adding Charles to CC.
> >
> > http://www.spinics.net/lists/linux-usb/msg116656.html
> >
> > > Reverting this
> > > breaks ethtool support in turn.
> >
> > Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> > this case on arndale). This is now a regression of functionality
> > from 3.17.
> >
> > I think it would better to revert the change now and with less hurry
> > introduce a ethtool fix that doesn't break arndale.
> >
> > > Kind regards,
> > >
> > > Michel Stam
> > >
> > > -----Original Message-----
> > > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > > Sent: Tuesday, November 04, 2014 8:23 AM
> > > To: davem@davemloft.net; Stam, Michel [FINT]
> > > Cc: linux-usb@vger.kernel.org; netdev@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> > > Subject: "asix: Don't reset PHY on if_up for ASIX 88772" breaks
> > > net on
> >
> > > arndale platform
> > >
> > > Hi,
> > >
> > > With 3.18-rc3, asix on arndale (samsung exynos 5250 based board),
> > > fails to work. Interface is initialized but network traffic seem
> > > not
>
> > > to pass through. With kernel IP config the result looks like:
> > >
> > > [ 3.323275] usb 3-3.2.4: new high-speed USB device number 4
using
> > > exynos-ehci
> > > [ 3.419151] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [ 3.424735] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [ 3.432196] usb 3-3.2.4: Product: AX88772
> > > [ 3.436279] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [ 3.441486] usb 3-3.2.4: SerialNumber: 000001
> > > [ 3.447530] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [ 3.764352] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > de:a2:66:bf:ca:4f
> > > [ 4.488773] asix 3-3.2.4:1.0 eth0: link down
> > > [ 5.690025] asix 3-3.2.4:1.0 eth0: link up, 100Mbps,
full-duplex,
> > lpa
> > > 0xC5E1
> > > [ 5.712947] Sending DHCP requests ...... timed out!
> > > [ 83.165303] IP-Config: Retrying forever (NFS root)...
> > > [ 83.170397] asix 3-3.2.4:1.0 eth0: link up, 100Mbps,
full-duplex,
> > lpa
> > > 0xC5E1
> > > [ 83.192944] Sending DHCP requests .....
> > >
> > > Similar results also with dhclient. Git bisect identified the
> > > breaking
> >
> > > commit as:
> > >
> > > commit 3cc81d85ee01e5a0b7ea2f4190e2ed1165f53c31
> > > Author: Michel Stam <m.stam@fugro.nl>
> > > Date: Thu Oct 2 10:22:02 2014 +0200
> > >
> > > asix: Don't reset PHY on if_up for ASIX 88772
> > >
> > > Taking 3.18-rc3 and that commit reverted, network works again:
> > >
> > > [ 3.303500] usb 3-3.2.4: new high-speed USB device number 4
using
> > > exynos-ehci
> > > [ 3.399375] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [ 3.404963] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [ 3.412424] usb 3-3.2.4: Product: AX88772
> > > [ 3.416508] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [ 3.421715] usb 3-3.2.4: SerialNumber: 000001
> > > [ 3.427755] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [ 3.744837] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > 12:59:f1:a8:43:90
> > > [ 7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps,
full-duplex,
> > lpa
> > > 0xC5E1
> > > [ 7.118258] Sending DHCP requests ., OK
> > > [ 9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my
> address
> > > is 192.168.1.111
> > >
> > > There might something wrong on the samsung platform code (I
> > > understand
> >
> > > the USB on arndale is "funny"), but this is still an regression
> > > from
>
> > > 3.17.
> > >
> > > Riku
^ permalink raw reply
* Re: mlx4+vxlan offload breaks gre tunnels
From: Or Gerlitz @ 2014-11-05 16:17 UTC (permalink / raw)
To: Florian Westphal, netdev, Tom Herbert, Jesse Gross; +Cc: amirv
In-Reply-To: <20141105150448.GA20776@breakpoint.cc>
On 11/5/2014 5:04 PM, Florian Westphal wrote:
> tl,dr: all tcp packets sent via gre tunnel have broken tcp csum if vxlan offload
> is enabled with mlx4 driver.
>
> Given following config on tx-side:
> dev=enp3s0
> ip addr add dev $dev 192.168.23.1/24
> ip link set $dev up
> ip link add mygre type gretap remote 192.168.23.2 local 192.168.23.1
> ip addr add dev mygre 192.168.42.1/24
> ip link set gre0 up
> ip link set mygre up
>
> and
>
> options mlx4_core log_num_mgm_entry_size=-1 debug_level=1
> port_type_array=2,2
>
> in
> /etc/modprobe.d/mlx4.conf
>
> all tcp packets sent to destinations over the gre tunnel have bogus tcp
> checksums (and are tossed on rx side when stack validates tcp checksum).
>
> net-next head is commit 30349bdbc4da5ecf0efa25556e3caff9c9b8c5f7 .
>
> What makes things work for me:
> either
>
> options mlx4_core 1 debug_level=1 port_type_array=2,2
>
> (ie. no MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
>
> or not setting NETIF_F_IP_CSUM in enc_features:
>
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -2579,10 +2579,12 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
> dev->priv_flags |= IFF_UNICAST_FLT;
>
> if (mdev->dev->caps.tunnel_offload_mode == MLX4_TUNNEL_OFFLOAD_MODE_VXLAN) {
> - dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
> + dev->hw_enc_features |= NETIF_F_RXCSUM |
> NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL;
>
> I am not sure if its right fix, but to my eyes this basically looks like
> mlx4 is telling stack that it can handle tcp checksum offload within
> tunnels, and that doesn't seem to be the case for all types (e.g. gre).
>
> Could someone who understand the enc_features specifics better confirm that
> above patch is correct (or provide a better/proper fix)?
Yep, I can see now the problem. It comes into play with ConnectX3-pro
NICs that support VXLAN offloads (but not with ConnectX3 NIC which
don't) when you enable the offloads support on the CX3-pro.
The problem originates from the fact that we can't advertize something
like "the HW can offload the inner checksum of UDP/VXLAN encapsulated
(but not for GRE)", e.g in a similar manner that exists in the GSO
space, where you have NETIF_F_GSO _YYY for each yyy in {UDP, SIT, GRE,
etc} tunneling scheme.
I think the best effort we can do now is
1. come up with something such as the below patch for 3.18 which is
back-ward portable for -stable kernels, it will only arm the hw offloads
if the OS tells us there's VXLAN in action
2. come up with proper kernel APIs to let NICs advertize which encap
schemes they can actually offload the inner checksum, Tom... your work
which now runs over netdev.
Tom/Jesse- thoughts? are you +1-ing the below approach?
Or.
tested to work with the following which is a bit different, tell me if
it works for you
# node A - with mlx4_en address192.168.31.18
ip tunnel add gre1 mode gre local 192.168.31.18 remote 192.168.31.17 ttl 255
ifconfig gre1 10.10.10.18/24 up
ifconfig gre1 mtu 1450
# node B - with mlx4_en address192.168.31.17
ip tunnel add gre1 mode gre local 192.168.31.17 remote 192.168.31.18 ttl 255
ifconfig gre1 10.10.10.17/24 up
ifconfig gre1 mtu 1450
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 0efbae9..7753833 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2292,6 +2292,12 @@ static void mlx4_en_add_vxlan_offloads(struct
work_struct *work)
out:
if (ret)
en_err(priv, "failed setting L2 tunnel configuration
ret %d\n", ret);
+
+ /* set offloads */
+ priv->dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
+ NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL;
+ priv->dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL;
+ priv->dev->features |= NETIF_F_GSO_UDP_TUNNEL;
}
static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
@@ -2299,6 +2305,10 @@ static void mlx4_en_del_vxlan_offloads(struct
work_struct *work)
int ret;
struct mlx4_en_priv *priv = container_of(work, struct mlx4_en_priv,
vxlan_del_task);
+ /* unset offloads */
+ priv->dev->hw_enc_features = 0;
+ priv->dev->hw_features &= ~NETIF_F_GSO_UDP_TUNNEL;
+ priv->dev->features &= ~NETIF_F_GSO_UDP_TUNNEL;
ret = mlx4_SET_PORT_VXLAN(priv->mdev->dev, priv->port,
VXLAN_STEER_BY_OUTER_MAC, 0);
@@ -2578,13 +2588,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev,
int port,
if (mdev->dev->caps.steering_mode != MLX4_STEERING_MODE_A0)
dev->priv_flags |= IFF_UNICAST_FLT;
- if (mdev->dev->caps.tunnel_offload_mode ==
MLX4_TUNNEL_OFFLOAD_MODE_VXLAN) {
- dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
- NETIF_F_TSO |
NETIF_F_GSO_UDP_TUNNEL;
- dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL;
- dev->features |= NETIF_F_GSO_UDP_TUNNEL;
- }
-
mdev->pndev[port] = dev;
netif_carrier_off(dev);
^ permalink raw reply related
* Re: [PATCH net v3] ipv6: mld: fix add_grhead skb_over_panic for devs with large MTUs
From: Eric Dumazet @ 2014-11-05 16:20 UTC (permalink / raw)
To: Daniel Borkmann
Cc: davem, lw1a2.jing, fw, hannes, netdev, Eric Dumazet,
David L Stevens
In-Reply-To: <1415198530-7126-1-git-send-email-dborkman@redhat.com>
On Wed, 2014-11-05 at 15:42 +0100, Daniel Borkmann wrote:
> It has been reported that generating an MLD listener report on
> devices with large MTUs (e.g. 9000) and a high number of IPv6
> addresses can trigger a skb_over_panic():
...
> v2->v3:
> - Still had a discussion w/ Hannes and improved the code a bit to
> make it more clear to read
I am very sorry Daniel, but I found v2 much easier to understand :(
Could you refrain from doing cleanups in this patch,
only provide the very minimal fix ?
No empty lines additions or deletions and stuff like that...
Then, we can cleanup for net-next later if you really want ;)
I know its _very_ tempting to do cleanups, but its very time consuming
to review patches having real stuff done (like bug fixes) and cleanups.
Thanks !
^ permalink raw reply
* RE: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net onarndale platform
From: Stam, Michel [FINT] @ 2014-11-05 16:21 UTC (permalink / raw)
To: Riku Voipio
Cc: Charles Keepax, davem, linux-usb, netdev, linux-kernel,
linux-samsung-soc
In-Reply-To: <20141105123937.GA5981@afflict.kos.to>
Hello Riku,
I will quickly reply to your message, and reserve any further comments
for the other thread.
>My concern is, that none of us with this problem is a linux network
drivers expert. And no such expert joined the thread to help us. Thus if
we hurry to have proper fix for 3.18, our fix might easily be really
wrong.
>
>Hence, it would seem safer to revert to 3.17 state before 3.18, so we
can propose a proper fix for 3.19. At least from our myopic view, having
no functioning net on arndale is worse than having non-functioning
ethtool (which doesn't >seem to have bothered people for years).
Lets agree to disagree here. Any further comment on this would not be
productive.
Kind regards,
Michel Stam
-----Original Message-----
From: Riku Voipio [mailto:riku.voipio@iki.fi]
Sent: Wednesday, November 05, 2014 13:40 PM
To: Stam, Michel [FINT]
Cc: Charles Keepax; davem@davemloft.net; linux-usb@vger.kernel.org;
netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
linux-samsung-soc@vger.kernel.org
Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net
onarndale platform
Hi Michel,
On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote:
> After looking around I found the reset value for the 8772 chip, which
> seems to be 0x1E1 (ANAR register).
>
> This equates to (according to include/uapi/linux/mii.h) ADVERTISE_ALL
> | ADVERTISE_CSMA.
>
> The register only seems to become 0 if the software reset fails.
>
> Unfortunately, this is exactly what I get when the patch is applied;
> asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5 asix
> 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2, ASIX
> AX88772 USB 2.0 Ethernet asix 1-2:1.0 eth1: Failed to send software
> reset: ffffffb5 asix 1-2:1.0 eth1: link reset failed (-75) usbnet
> usb-0000:00:1d.0-2, ASIX AX88772 USB 2.0 Ethernet
>
> A little while after this its 'Failed to enable software MII access'.
> The ethernet device fails to see any link or accept any ethtool -s
> command.
> My device has vid:devid 0b95:772a (ASIX Elec. Corp.).
> Can you tell me what device is on the Andale platform, Charles? Same
> vendor/device id?
Bus 003 Device 004: ID 0b95:772a ASIX Electronics Corp. AX88772A Fast
Ethernet
> Another thing that bothers me is that dev->mii.advertising seems to
> contain the same value, so maybe that can be used instead of a call to
> asix_mdio_read(). Can anyone comment on its purpose? Should it be a
> shadow copy of the real register or something?
> Riku, can you test Charles' patch as well?
With that patch + revert to ax88772_reset network works. I'm unable to
get ethtool to work with that patch or with the original 3.17 state of
asix.
Once i disable autoneg network doesn't just work.
> > >I think it would better to revert the change now and with less
> > >hurry
> > introduce a ethtool fix that doesn't break arndale.
> > I don't fully agree here;
> > I would like to point out that this commit is a revert itself.
> > Fixing the armdale will then cause breakage in other
> > implementations, such as ours. Blankly reverting breaks other
peoples' implementations.
My concern is, that none of us with this problem is a linux network
drivers expert. And no such expert joined the thread to help us. Thus if
we hurry to have proper fix for 3.18, our fix might easily be really
wrong.
Hence, it would seem safer to revert to 3.17 state before 3.18, so we
can propose a proper fix for 3.19. At least from our myopic view, having
no functioning net on arndale is worse than having non-functioning
ethtool (which doesn't seem to have bothered people for years).
Riku
> > The PHY reset is the thing that breaks ethtool support, so any fix
> > that appeases all would have to take existing PHY state into
account.
> >
> > I'm not an expert on the ASIX driver, nor the MII, but I think this
> > is
>
> > the cause;
> > drivers/net/usb/asix_devices.c:
> > 361 asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> > BMCR_RESET);
> > 362 asix_mdio_write(dev->net, dev->mii.phy_id,
MII_ADVERTISE,
> > 363 ADVERTISE_ALL | ADVERTISE_CSMA);
> > 364 mii_nway_restart(&dev->mii);
> >
> > I would think that the ADVERTISE_ALL is the cause here, as it will
> > reset the MII back to default, thus overriding ethtool settings.
> > Would an:
> > Int reg;
> > reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
> >
> > prior to the offending lines, and then;
> >
> > 362 asix_mdio_write(dev->net, dev->mii.phy_id,
MII_ADVERTISE,
> > 363 reg);
> >
> > solve the problem for you guys?
>
> If I revert the patch in question and add this in:
>
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev) {
> struct asix_data *data = (struct asix_data *)&dev->data;
> int ret, embd_phy;
> + int reg;
> u16 rx_ctl;
>
> ret = asix_write_gpio(dev,
> @@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
> msleep(150);
>
> asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> BMCR_RESET);
> - asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> - ADVERTISE_ALL | ADVERTISE_CSMA);
> + reg = asix_mdio_read(dev->net, dev->mii.phy_id,
MII_ADVERTISE);
> + if (!reg)
> + reg = ADVERTISE_ALL | ADVERTISE_CSMA;
> + asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> + reg);
> mii_nway_restart(&dev->mii);
>
> ret = asix_write_medium_mode(dev, AX88772_MEDIUM_DEFAULT);
>
> Then things work on Arndale for me. Does that work for you?
> Whether that is a sensible fix I don't know however.
>
> >
> > Mind, maybe the read function should take into account the reset
> > value
>
> > of the MII, and set it to ADVERTISE_ALL | ADVERTISE_CSMA. I don't
> > have
>
> > any documention here at the moment.
>
> Yeah I also have no documentation.
>
> Thanks,
> Charles
>
> >
> > Is anyone able to confirm my suspicions?
> >
> > Kind regards,
> >
> > Michel Stam
> > -----Original Message-----
> > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > Sent: Tuesday, November 04, 2014 10:44 AM
> > To: Stam, Michel [FINT]
> > Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org;
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux-samsung-soc@vger.kernel.org;
> > ckeepax@opensource.wolfsonmicro.com
> > Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks
> > net on arndale platform
> >
> > On Tue, Nov 04, 2014 at 09:19:26AM +0100, Stam, Michel [FINT] wrote:
> > > Interesting, as the commit itself is a revert from a kernel back
> > > to
> > > 2.6 somewhere. The problem I had is related to the PHY being reset
> > > on interface-up, can you confirm that you require this?
> >
> > I can't confirm what exactly is needed on arndale. I'm neither
> > expert in USB or ethernet. However, I can confirm that without the
> > PHY reset,
>
> > networking doesn't work on arndale.
> >
> > I now see someone else has the same problem, adding Charles to CC.
> >
> > http://www.spinics.net/lists/linux-usb/msg116656.html
> >
> > > Reverting this
> > > breaks ethtool support in turn.
> >
> > Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> > this case on arndale). This is now a regression of functionality
> > from 3.17.
> >
> > I think it would better to revert the change now and with less hurry
> > introduce a ethtool fix that doesn't break arndale.
> >
> > > Kind regards,
> > >
> > > Michel Stam
> > >
> > > -----Original Message-----
> > > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > > Sent: Tuesday, November 04, 2014 8:23 AM
> > > To: davem@davemloft.net; Stam, Michel [FINT]
> > > Cc: linux-usb@vger.kernel.org; netdev@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> > > Subject: "asix: Don't reset PHY on if_up for ASIX 88772" breaks
> > > net on
> >
> > > arndale platform
> > >
> > > Hi,
> > >
> > > With 3.18-rc3, asix on arndale (samsung exynos 5250 based board),
> > > fails to work. Interface is initialized but network traffic seem
> > > not
>
> > > to pass through. With kernel IP config the result looks like:
> > >
> > > [ 3.323275] usb 3-3.2.4: new high-speed USB device number 4
using
> > > exynos-ehci
> > > [ 3.419151] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [ 3.424735] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [ 3.432196] usb 3-3.2.4: Product: AX88772
> > > [ 3.436279] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [ 3.441486] usb 3-3.2.4: SerialNumber: 000001
> > > [ 3.447530] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [ 3.764352] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > de:a2:66:bf:ca:4f
> > > [ 4.488773] asix 3-3.2.4:1.0 eth0: link down
> > > [ 5.690025] asix 3-3.2.4:1.0 eth0: link up, 100Mbps,
full-duplex,
> > lpa
> > > 0xC5E1
> > > [ 5.712947] Sending DHCP requests ...... timed out!
> > > [ 83.165303] IP-Config: Retrying forever (NFS root)...
> > > [ 83.170397] asix 3-3.2.4:1.0 eth0: link up, 100Mbps,
full-duplex,
> > lpa
> > > 0xC5E1
> > > [ 83.192944] Sending DHCP requests .....
> > >
> > > Similar results also with dhclient. Git bisect identified the
> > > breaking
> >
> > > commit as:
> > >
> > > commit 3cc81d85ee01e5a0b7ea2f4190e2ed1165f53c31
> > > Author: Michel Stam <m.stam@fugro.nl>
> > > Date: Thu Oct 2 10:22:02 2014 +0200
> > >
> > > asix: Don't reset PHY on if_up for ASIX 88772
> > >
> > > Taking 3.18-rc3 and that commit reverted, network works again:
> > >
> > > [ 3.303500] usb 3-3.2.4: new high-speed USB device number 4
using
> > > exynos-ehci
> > > [ 3.399375] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > > idProduct=772a
> > > [ 3.404963] usb 3-3.2.4: New USB device strings: Mfr=1,
> Product=2,
> > > SerialNumber=3
> > > [ 3.412424] usb 3-3.2.4: Product: AX88772
> > > [ 3.416508] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > > [ 3.421715] usb 3-3.2.4: SerialNumber: 000001
> > > [ 3.427755] asix 3-3.2.4:1.0 (unnamed net_device)
> (uninitialized):
> > > invalid hw address, using random
> > > [ 3.744837] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> > 12:59:f1:a8:43:90
> > > [ 7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps,
full-duplex,
> > lpa
> > > 0xC5E1
> > > [ 7.118258] Sending DHCP requests ., OK
> > > [ 9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my
> address
> > > is 192.168.1.111
> > >
> > > There might something wrong on the samsung platform code (I
> > > understand
> >
> > > the USB on arndale is "funny"), but this is still an regression
> > > from
>
> > > 3.17.
> > >
> > > Riku
^ permalink raw reply
* Re: [PATCH V3 1/3] can: add can_is_canfd_skb() API
From: Eric Dumazet @ 2014-11-05 16:22 UTC (permalink / raw)
To: Dong Aisheng
Cc: linux-can, mkl, wg, varkabhadram, netdev, socketcan,
linux-arm-kernel
In-Reply-To: <1415193393-30023-1-git-send-email-b29396@freescale.com>
On Wed, 2014-11-05 at 21:16 +0800, Dong Aisheng wrote:
> The CAN device drivers can use it to check if the frame to send is on
> CAN FD mode or normal CAN mode.
>
> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> ---
> ChangesLog:
> * v1->v2: change to skb->len == CANFD_MTU;
> ---
> include/linux/can/dev.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
> index 6992afc..4c3919c 100644
> --- a/include/linux/can/dev.h
> +++ b/include/linux/can/dev.h
> @@ -99,6 +99,11 @@ inval_skb:
> return 1;
> }
>
This looks a bit strange to assume that skb->len == magical_value is CAN
FD. A comment would be nice.
> +static inline int can_is_canfd_skb(struct sk_buff *skb)
static inline bool can_is_canfd_skb(const struct sk_buff *skb)
> +{
> + return skb->len == CANFD_MTU;
> +}
> +
> /* get data length from can_dlc with sanitized can_dlc */
> u8 can_dlc2len(u8 can_dlc);
>
^ permalink raw reply
* Fwd: Kernel Oops in __inet_twsk_kill()
From: Daniel Borkmann @ 2014-11-05 16:00 UTC (permalink / raw)
To: charley.chu; +Cc: netdev
In-Reply-To: <FB8A4655DFD2B34DB16AE06DDDD6C0E231A6F030@SJEXCHMB12.corp.ad.broadcom.com>
[ moving to netdev ]
-------- Original Message --------
Subject: Kernel Oops in __inet_twsk_kill()
Date: Tue, 4 Nov 2014 23:47:18 +0000
From: Charley (Hao Chuan) Chu <charley.chu@broadcom.com>
To: linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
We have situation on our system. It brings the network interface up and down every
a few seconds. Eventually, it brings down the system - the kernel crashed due to BUG
on in __inet_twsk_kill(). The debug message show following call flow.
1) time-wait socket is created by tcp_time_wait() when the socket gets into "TIME_WAIT" state.
inet_twsk_alloc() - refcnt= 0
inet_twsk_hashdance() - refcnt = 3
inet_twsk_schedule() - refcnt = 4
inet_twsk_put() - refcnt = 3
2) tcp_v4_timewait_ack() is called when sync is received
inet_twsk_put() - refcnt= 2 <== where we thing the problem is
occasionally, second sync is received, so the inet_twsk_put is called twice - refcnt = 1
3) twdr_do_twkill_work() is called when timed out
call __inet_twsk_kill - BUG_ON!!! as refcnt=2 (supposed to be 3).
call inet_twsk_put()
In a normal case, the callflow only has step 1 and step 3. Our understanding is
the time-wait socket has three references - ehash, bhash and timer death row. In
step 2, none of them are touched. Can anyone here explain to us why the inet_twsk_put()
is called in tcp_v4_timewait_ack()?
our system has 3.14 kernel.
Any help would be highly appreciated.
Charley Chu
^ permalink raw reply
* Re: [PATCH net v3] ipv6: mld: fix add_grhead skb_over_panic for devs with large MTUs
From: Daniel Borkmann @ 2014-11-05 16:26 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, lw1a2.jing, fw, hannes, netdev, Eric Dumazet,
David L Stevens
In-Reply-To: <1415204413.13896.5.camel@edumazet-glaptop2.roam.corp.google.com>
On 11/05/2014 05:20 PM, Eric Dumazet wrote:
> On Wed, 2014-11-05 at 15:42 +0100, Daniel Borkmann wrote:
>> It has been reported that generating an MLD listener report on
>> devices with large MTUs (e.g. 9000) and a high number of IPv6
>> addresses can trigger a skb_over_panic():
> ...
>> v2->v3:
>> - Still had a discussion w/ Hannes and improved the code a bit to
>> make it more clear to read
>
> I am very sorry Daniel, but I found v2 much easier to understand :(
>
> Could you refrain from doing cleanups in this patch,
> only provide the very minimal fix ?
>
> No empty lines additions or deletions and stuff like that...
>
> Then, we can cleanup for net-next later if you really want ;)
>
> I know its _very_ tempting to do cleanups, but its very time consuming
> to review patches having real stuff done (like bug fixes) and cleanups.
I can understand, sorry, I'm fine with either version actually.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net v3] ipv6: mld: fix add_grhead skb_over_panic for devs with large MTUs
From: Hannes Frederic Sowa @ 2014-11-05 16:38 UTC (permalink / raw)
To: Eric Dumazet, Daniel Borkmann
Cc: davem, lw1a2.jing, fw, netdev, Eric Dumazet, David L Stevens
In-Reply-To: <1415204413.13896.5.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, Nov 5, 2014, at 17:20, Eric Dumazet wrote:
> On Wed, 2014-11-05 at 15:42 +0100, Daniel Borkmann wrote:
> > It has been reported that generating an MLD listener report on
> > devices with large MTUs (e.g. 9000) and a high number of IPv6
> > addresses can trigger a skb_over_panic():
>
> ...
>
> > v2->v3:
> > - Still had a discussion w/ Hannes and improved the code a bit to
> > make it more clear to read
>
> I am very sorry Daniel, but I found v2 much easier to understand :(
>
> Could you refrain from doing cleanups in this patch,
> only provide the very minimal fix ?
>
> No empty lines additions or deletions and stuff like that...
>
> Then, we can cleanup for net-next later if you really want ;)
>
> I know its _very_ tempting to do cleanups, but its very time consuming
> to review patches having real stuff done (like bug fixes) and cleanups.
My point was that the max_t(int, ..., ...) assignment to
reserved_tailroom was too implicit in case we allocated an skb smaller
than the mtu and reserved_tailroom should become '0'.
I would still vote for this version, but see the problem with the noise
caused by newline updates. Eric, would you mind a new version with only
the essential parts changed and keeping this calculation so we don't
need to change it twice for net and for net-next?
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH net v3] ipv6: mld: fix add_grhead skb_over_panic for devs with large MTUs
From: Eric Dumazet @ 2014-11-05 16:48 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Daniel Borkmann, davem, lw1a2.jing, fw, netdev, Eric Dumazet,
David L Stevens
In-Reply-To: <1415205483.3264462.187432293.00A1F284@webmail.messagingengine.com>
On Wed, 2014-11-05 at 17:38 +0100, Hannes Frederic Sowa wrote:
> I would still vote for this version, but see the problem with the noise
> caused by newline updates. Eric, would you mind a new version with only
> the essential parts changed and keeping this calculation so we don't
> need to change it twice for net and for net-next?
I will be happy to review a v4 ;)
Thanks !
^ permalink raw reply
* Re: mlx4+vxlan offload breaks gre tunnels
From: Florian Westphal @ 2014-11-05 16:53 UTC (permalink / raw)
To: Or Gerlitz; +Cc: Florian Westphal, netdev, Tom Herbert, Jesse Gross, amirv
In-Reply-To: <545A4DB7.5010603@mellanox.com>
Or Gerlitz <ogerlitz@mellanox.com> wrote:
> On 11/5/2014 5:04 PM, Florian Westphal wrote:
> >tl,dr: all tcp packets sent via gre tunnel have broken tcp csum if vxlan offload
> >is enabled with mlx4 driver.
> >
> Yep, I can see now the problem. It comes into play with
> ConnectX3-pro NICs that support VXLAN offloads (but not with
> ConnectX3 NIC which don't) when you enable the offloads support on
> the CX3-pro.
[..]
> I think the best effort we can do now is
>
> 1. come up with something such as the below patch for 3.18 which is
> back-ward portable for -stable kernels, it will only arm the hw
> offloads if the OS tells us there's VXLAN in action
>
[..]
>
> tested to work with the following which is a bit different, tell me
> if it works for you
Right, the patch below works in my setup as well (until link-add-vxlan,
that is ;) )
Thanks,
Florian
^ permalink raw reply
* Re: [Xen-devel] [PATCHv1 net-next] xen-netback: remove unconditional pull_skb_tail in guest Tx path
From: David Miller @ 2014-11-05 17:15 UTC (permalink / raw)
To: Ian.Campbell
Cc: zoltan.kiss, david.vrabel, netdev, malcolm.crossley, wei.liu2,
xen-devel
In-Reply-To: <1415181080.11486.63.camel@citrix.com>
From: Ian Campbell <Ian.Campbell@citrix.com>
Date: Wed, 5 Nov 2014 09:51:20 +0000
> Is this also true for things which hit the iptables paths? I suppose
> they must necessarily have already been through the protocol demux stage
> before iptables would even be able to interpret them as e.g. an IP
> packet.
Netfilter often takes a different approach, by using
skb_header_pointer() which returns a direct pointer if the linear area
contains the requested range already, or alternatively copies from the
frags into a user supplied on-stack header buffer if not.
^ permalink raw reply
* Re: [PATCHv1 net-next] xen-netback: remove unconditional pull_skb_tail in guest Tx path
From: David Miller @ 2014-11-05 17:16 UTC (permalink / raw)
To: Ian.Campbell; +Cc: david.vrabel, netdev, xen-devel, wei.liu2, malcolm.crossley
In-Reply-To: <1415181185.11486.65.camel@citrix.com>
From: Ian Campbell <Ian.Campbell@citrix.com>
Date: Wed, 5 Nov 2014 09:53:05 +0000
> I'd like to see the commit message expanded to explain why this isn't
> introducing a (security) bug by not pulling enough stuff into the header
> (IOW the conclusion of the discussion).
Just because a fundamental aspect of how we handle packets isn't clear
to some people, doesn't mean that every commit that depends upon that
invariant has to explain it in the commit message. :-)
^ permalink raw reply
* Re: [PATCH V3 1/3] can: add can_is_canfd_skb() API
From: Oliver Hartkopp @ 2014-11-05 17:33 UTC (permalink / raw)
To: Eric Dumazet, Dong Aisheng
Cc: linux-can, mkl, wg, varkabhadram, netdev, linux-arm-kernel
In-Reply-To: <1415204533.13896.7.camel@edumazet-glaptop2.roam.corp.google.com>
On 05.11.2014 17:22, Eric Dumazet wrote:
> On Wed, 2014-11-05 at 21:16 +0800, Dong Aisheng wrote:
>
> This looks a bit strange to assume that skb->len == magical_value is CAN
> FD. A comment would be nice.
>
Yes. Due to exactly two types of struct can(fd)_frame which can be contained
in a skb the skbs are distinguished by the length which can be either CAN_MTU
or CANFD_MTU.
>> +static inline int can_is_canfd_skb(struct sk_buff *skb)
>
> static inline bool can_is_canfd_skb(const struct sk_buff *skb)
>
ok.
>> +{
What about:
/* the CAN specific type of skb is identified by its data length */
>> + return skb->len == CANFD_MTU;
>> +}
>> +
>> /* get data length from can_dlc with sanitized can_dlc */
>> u8 can_dlc2len(u8 can_dlc);
Regards,
Oliver
^ permalink raw reply
* Re: [PATCH net v3] ipv6: mld: fix add_grhead skb_over_panic for devs with large MTUs
From: Daniel Borkmann @ 2014-11-05 17:59 UTC (permalink / raw)
To: Eric Dumazet
Cc: Hannes Frederic Sowa, davem, lw1a2.jing, fw, netdev, Eric Dumazet,
David L Stevens
In-Reply-To: <1415206106.13896.8.camel@edumazet-glaptop2.roam.corp.google.com>
On 11/05/2014 05:48 PM, Eric Dumazet wrote:
> On Wed, 2014-11-05 at 17:38 +0100, Hannes Frederic Sowa wrote:
...
>> I would still vote for this version, but see the problem with the noise
>> caused by newline updates. Eric, would you mind a new version with only
>> the essential parts changed and keeping this calculation so we don't
>> need to change it twice for net and for net-next?
>
> I will be happy to review a v4 ;)
No problem, I'll respin. ;)
Thanks,
Daniel
^ 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