* Re: [PATCH RFC 0/4] Fixes for Marvell MII paged register access races
From: Russell King - ARM Linux @ 2017-12-09 23:49 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Andrew Lunn, netdev
In-Reply-To: <a968b765-a9e9-9ff7-9a95-2a98ce0db9fc@gmail.com>
On Sat, Dec 09, 2017 at 10:22:58AM -0800, Florian Fainelli wrote:
> On 12/08/2017 08:44 AM, Russell King - ARM Linux wrote:
> > On Fri, Dec 08, 2017 at 05:17:14PM +0100, Andrew Lunn wrote:
> >> Hi Russell
> >>
> >>> There is an open question whether there should be generic helpers for
> >>> this. Generic helpers would mean:
> >>>
> >>> - Additional couple of function pointers in phy_driver to read/write the
> >>> paging register. This has the restriction that there must only be one
> >>> paging register.
> >>
> >> I must be missing something. I don't see why there is this
> >> restriction. Don't we just need
> >>
> >> int phy_get_page(phydev);
> >> int phy_set_page(phydev, page);
> >
> > The restriction occurs because a PHY may have several different
> > registers, and knowing which of the registers need touching becomes an
> > issue. We wouldn't want these accessors to needlessly access several
> > registers each and every time we requested an access to the page
> > register.
> >
> > There's also the issue of whether an "int" or whatever type we choose to
> > pass the "page" around is enough bits. I haven't surveyed all the PHY
> > drivers yet to know the answer to that.
>
> I have not come across a PHY yet that required writing a page across two
> 16-bit quantities, in general, the page fits within less than 16-bit
> actually to fit within one MDIO write. That does not mean it cannot
> exist obviously, but having about 32-bit x pages of address space within
> a PHY sounds a bit extreme.
True, and phylib at the moment contains nothing beyond a single register.
I was thinking more of paging bits across several registers - such a case
would not lend itself well to this implementation as you'd have to read
every paging-capable register and write every paging capable register in
the phy_driver page accessor methods.
The good news is, having read through several drivers that contain the
caseless "page" string, there are no drivers that need anything but a
simple paging case, so it's not a concern. Those which seem to use
page accesses are:
at803x: this only uses a single bit in a register for one access.
dp83640: looks like it implements its own locking and banks registers
0x10-0x1e. Multiple accesses throughout the driver.
marvell: we know about this one which is the problem case.
microchip: looks like it banks the registers 0x10-0x1e, and uses this
for mdix control.
mscc: looks like it banks the registers 0x10-0x1e. Several accesses
throughout the driver, some under the phydev lock but others
unclear whether they are locked. Could be a problem.
realtek: looks like it banks the registers 0x10-0x1e. Probably racy -
interrupt handling uses paged accesses which may run in a
threaded interrupt handler.
vitesse: "/* map extended registers set 0x10 - 0x1e */" in one place
for mdix control via config_aneg.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* Re: [PATCH net-next] libbpf: add function to setup XDP
From: Jakub Kicinski @ 2017-12-09 23:57 UTC (permalink / raw)
To: Eric Leblond; +Cc: netdev, linux-kernel, ast, daniel
In-Reply-To: <20171209144315.25890-1-eric@regit.org>
On Sat, 9 Dec 2017 15:43:15 +0100, Eric Leblond wrote:
> + for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
> + nh = NLMSG_NEXT(nh, len)) {
> + if (nh->nlmsg_pid != getpid()) {
> + ret = -LIBBPF_ERRNO__WRNGPID;
> + goto cleanup;
> + }
> + if (nh->nlmsg_seq != seq) {
> + ret = -LIBBPF_ERRNO__INVSEQ;
> + goto cleanup;
> + }
> + switch (nh->nlmsg_type) {
> + case NLMSG_ERROR:
> + err = (struct nlmsgerr *)NLMSG_DATA(nh);
> + if (!err->error)
> + continue;
> + ret = err->error;
> + goto cleanup;
> + case NLMSG_DONE:
> + break;
> + default:
> + break;
> + }
Would it be possible to print out or preferably return to the caller
the ext ack error message? A couple of drivers are using it for XDP
mis-configuration reporting instead of printks. We should encourage
other to do the same and support it in all user space since ext ack
msgs lead to much better user experience.
^ permalink raw reply
* Re: [PATCH RFC 0/4] Fixes for Marvell MII paged register access races
From: Andrew Lunn @ 2017-12-10 0:19 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: Florian Fainelli, netdev
In-Reply-To: <20171209234905.GL10595@n2100.armlinux.org.uk>
> The good news is, having read through several drivers that contain the
> caseless "page" string, there are no drivers that need anything but a
> simple paging case, so it's not a concern.
Hi Russell
Also grep for bank. rockchip.c.
Andrew
^ permalink raw reply
* Re: [PATCH net-next] veth: fix setting peer gso values
From: Solio Sarabia @ 2017-12-10 1:16 UTC (permalink / raw)
To: davem, stephen; +Cc: netdev, sthemmin, shiny.sebastian, solio.sarabia
In-Reply-To: <1512846693-19983-1-git-send-email-solio.sarabia@intel.com>
On Sat, Dec 09, 2017 at 11:11:33AM -0800, Solio Sarabia wrote:
> GSO values are not correctly set for peer and dev. When creating
> the veth link, GSO attributes are set for peer device, propagate
> these values from peer to dev.
>
> Signed-off-by: Solio Sarabia <solio.sarabia@intel.com>
> ---
> drivers/net/veth.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index a69ad39..d936c8a 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -410,8 +410,8 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
> if (ifmp && (dev->ifindex != 0))
> peer->ifindex = ifmp->ifi_index;
>
> - peer->gso_max_size = dev->gso_max_size;
> - peer->gso_max_segs = dev->gso_max_segs;
> + dev->gso_max_size = peer->gso_max_size;
> + dev->gso_max_segs = peer->gso_max_segs;
Seems this patch won't be necessary after all. Parameter order in
iproute2 util will be changed to set GSO on first dev device (not
on the peer), making current code right: cloning from dev to peer.
>
> err = register_netdevice(peer);
> put_net(net);
> --
> 2.7.4
>
^ permalink raw reply
* [PATCHv2 net-next 1/1] forcedeth: remove unnecessary structure member
From: Zhu Yanjun @ 2017-12-10 3:07 UTC (permalink / raw)
To: yanjun.zhu, netdev, keescook, davem
In-Reply-To: <20171207.140719.544312559507472280.davem@davemloft.net>
Since both tx_ring and first_tx are the head of tx ring, it not
necessary to use two structure members to statically indicate
the head of tx ring. So first_tx is removed.
CC: Srinivas Eeda <srinivas.eeda@oracle.com>
CC: Joe Jin <joe.jin@oracle.com>
CC: Junxiao Bi <junxiao.bi@oracle.com>
Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
---
drivers/net/ethernet/nvidia/forcedeth.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 53614ed..cadea67 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -822,7 +822,7 @@ struct fe_priv {
/*
* tx specific fields.
*/
- union ring_type get_tx, put_tx, first_tx, last_tx;
+ union ring_type get_tx, put_tx, last_tx;
struct nv_skb_map *get_tx_ctx, *put_tx_ctx;
struct nv_skb_map *first_tx_ctx, *last_tx_ctx;
struct nv_skb_map *tx_skb;
@@ -1932,7 +1932,8 @@ static void nv_init_tx(struct net_device *dev)
struct fe_priv *np = netdev_priv(dev);
int i;
- np->get_tx = np->put_tx = np->first_tx = np->tx_ring;
+ np->get_tx = np->tx_ring;
+ np->put_tx = np->tx_ring;
if (!nv_optimized(np))
np->last_tx.orig = &np->tx_ring.orig[np->tx_ring_size-1];
@@ -2248,7 +2249,7 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
offset += bcnt;
size -= bcnt;
if (unlikely(put_tx++ == np->last_tx.orig))
- put_tx = np->first_tx.orig;
+ put_tx = np->tx_ring.orig;
if (unlikely(np->put_tx_ctx++ == np->last_tx_ctx))
np->put_tx_ctx = np->first_tx_ctx;
} while (size);
@@ -2294,13 +2295,13 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
offset += bcnt;
frag_size -= bcnt;
if (unlikely(put_tx++ == np->last_tx.orig))
- put_tx = np->first_tx.orig;
+ put_tx = np->tx_ring.orig;
if (unlikely(np->put_tx_ctx++ == np->last_tx_ctx))
np->put_tx_ctx = np->first_tx_ctx;
} while (frag_size);
}
- if (unlikely(put_tx == np->first_tx.orig))
+ if (unlikely(put_tx == np->tx_ring.orig))
prev_tx = np->last_tx.orig;
else
prev_tx = put_tx - 1;
@@ -2406,7 +2407,7 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
offset += bcnt;
size -= bcnt;
if (unlikely(put_tx++ == np->last_tx.ex))
- put_tx = np->first_tx.ex;
+ put_tx = np->tx_ring.ex;
if (unlikely(np->put_tx_ctx++ == np->last_tx_ctx))
np->put_tx_ctx = np->first_tx_ctx;
} while (size);
@@ -2452,13 +2453,13 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
offset += bcnt;
frag_size -= bcnt;
if (unlikely(put_tx++ == np->last_tx.ex))
- put_tx = np->first_tx.ex;
+ put_tx = np->tx_ring.ex;
if (unlikely(np->put_tx_ctx++ == np->last_tx_ctx))
np->put_tx_ctx = np->first_tx_ctx;
} while (frag_size);
}
- if (unlikely(put_tx == np->first_tx.ex))
+ if (unlikely(put_tx == np->tx_ring.ex))
prev_tx = np->last_tx.ex;
else
prev_tx = put_tx - 1;
@@ -2597,7 +2598,7 @@ static int nv_tx_done(struct net_device *dev, int limit)
}
}
if (unlikely(np->get_tx.orig++ == np->last_tx.orig))
- np->get_tx.orig = np->first_tx.orig;
+ np->get_tx.orig = np->tx_ring.orig;
if (unlikely(np->get_tx_ctx++ == np->last_tx_ctx))
np->get_tx_ctx = np->first_tx_ctx;
}
@@ -2651,7 +2652,7 @@ static int nv_tx_done_optimized(struct net_device *dev, int limit)
}
if (unlikely(np->get_tx.ex++ == np->last_tx.ex))
- np->get_tx.ex = np->first_tx.ex;
+ np->get_tx.ex = np->tx_ring.ex;
if (unlikely(np->get_tx_ctx++ == np->last_tx_ctx))
np->get_tx_ctx = np->first_tx_ctx;
}
--
2.7.4
^ permalink raw reply related
* [PATCH] net: ipv4: fix for a race condition in raw_sendmsg
From: simo.ghannam @ 2017-12-10 3:50 UTC (permalink / raw)
To: netdev; +Cc: Mohamed Ghannam
From: Mohamed Ghannam <simo.ghannam@gmail.com>
inet->hdrincl is racy, and could lead to uninitialized stack pointer
usage, so its value should be read only once.
Signed-off-by: Mohamed Ghannam <simo.ghannam@gmail.com>
---
net/ipv4/raw.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 33b70bfd1122..125c1eab3eaa 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -513,11 +513,16 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
int err;
struct ip_options_data opt_copy;
struct raw_frag_vec rfv;
+ int hdrincl;
err = -EMSGSIZE;
if (len > 0xFFFF)
goto out;
+ /* hdrincl should be READ_ONCE(inet->hdrincl)
+ * but READ_ONCE() doesn't work with bit fields
+ */
+ hdrincl = inet->hdrincl;
/*
* Check the flags.
*/
@@ -593,7 +598,7 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
/* Linux does not mangle headers on raw sockets,
* so that IP options + IP_HDRINCL is non-sense.
*/
- if (inet->hdrincl)
+ if (hdrincl)
goto done;
if (ipc.opt->opt.srr) {
if (!daddr)
@@ -615,12 +620,12 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
flowi4_init_output(&fl4, ipc.oif, sk->sk_mark, tos,
RT_SCOPE_UNIVERSE,
- inet->hdrincl ? IPPROTO_RAW : sk->sk_protocol,
+ hdrincl ? IPPROTO_RAW : sk->sk_protocol,
inet_sk_flowi_flags(sk) |
- (inet->hdrincl ? FLOWI_FLAG_KNOWN_NH : 0),
+ (hdrincl ? FLOWI_FLAG_KNOWN_NH : 0),
daddr, saddr, 0, 0, sk->sk_uid);
- if (!inet->hdrincl) {
+ if (!hdrincl) {
rfv.msg = msg;
rfv.hlen = 0;
@@ -645,7 +650,7 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
goto do_confirm;
back_from_confirm:
- if (inet->hdrincl)
+ if (hdrincl)
err = raw_send_hdrinc(sk, &fl4, msg, len,
&rt, msg->msg_flags, &ipc.sockc);
--
2.14.1
^ permalink raw reply related
* Re: [PATCH] net: ipv4: fix for a race condition in raw_sendmsg
From: Eric Dumazet @ 2017-12-10 4:30 UTC (permalink / raw)
To: simo.ghannam, netdev; +Cc: Herbert Xu
In-Reply-To: <5a2caf2e.4ce61c0a.5017a.575f@mx.google.com>
On Sun, 2017-12-10 at 03:50 +0000, simo.ghannam@gmail.com wrote:
> From: Mohamed Ghannam <simo.ghannam@gmail.com>
>
> inet->hdrincl is racy, and could lead to uninitialized stack pointer
> usage, so its value should be read only once.
>
> Signed-off-by: Mohamed Ghannam <simo.ghannam@gmail.com>
> ---
> net/ipv4/raw.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
Thanks a lot for fixing this very serious bug.
Reviewed-by: Eric Dumazet <edumazet@google.com>
Please David add :
Fixes: c008ba5bdc9f ("ipv4: Avoid reading user iov twice after raw_probe_proto_opt")
Thanks !
^ permalink raw reply
* Re: kernel BUG at net/core/skbuff.c:LINE! (2)
From: Xin Long @ 2017-12-10 4:36 UTC (permalink / raw)
To: Eric Dumazet
Cc: syzbot, davem, kuznet, LKML, linux-sctp, network dev, Neil Horman,
syzkaller-bugs, Vlad Yasevich, yoshfuji
In-Reply-To: <1512838770.25033.33.camel@gmail.com>
On Sun, Dec 10, 2017 at 12:59 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2017-12-09 at 19:23 +0800, Xin Long wrote:
>> On Fri, Dec 8, 2017 at 4:45 PM, Xin Long <lucien.xin@gmail.com>
>> wrote:
>> > On Fri, Dec 8, 2017 at 4:16 PM, syzbot
>> > <bot+ed0838d0fa4c4f2b528e20286e6dc63effc7c14d@syzkaller.appspotmail
>> > .com>
>> > wrote:
>> > > syzkaller has found reproducer for the following crash on
>> > > 82bcf1def3b5f1251177ad47c44f7e17af039b4b
>> > > git://git.cmpxchg.org/linux-mmots.git/master
>> > > compiler: gcc (GCC) 7.1.1 20170620
>> > > .config is attached
>> > > Raw console output is attached.
>> > >
>> > > syzkaller reproducer is attached. See https://goo.gl/kgGztJ
>> > > for information about syzkaller reproducers
>> > >
>> > >
>> > > skbuff: skb_over_panic: text:0000000010b86b8d len:196 put:20
>> > > head:000000003b477e60 data:000000000e85441e tail:0xd4 end:0xc0
>> > > dev:lo
>> > > ------------[ cut here ]------------
>> > > kernel BUG at net/core/skbuff.c:104!
>> > > invalid opcode: 0000 [#1] SMP KASAN
>> > > Dumping ftrace buffer:
>> > > (ftrace buffer empty)
>> > > Modules linked in:
>> > > CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.15.0-rc2-mm1+ #39
>> > > Hardware name: Google Google Compute Engine/Google Compute
>> > > Engine, BIOS
>> > > Google 01/01/2011
>> > > RIP: 0010:skb_panic+0x15c/0x1f0 net/core/skbuff.c:100
>> > > RSP: 0018:ffff8801db307508 EFLAGS: 00010286
>> > > RAX: 0000000000000082 RBX: ffff8801c517e840 RCX: 0000000000000000
>> > > RDX: 0000000000000082 RSI: 1ffff1003b660e61 RDI: ffffed003b660e95
>> > > RBP: ffff8801db307570 R08: 1ffff1003b660e23 R09: 0000000000000000
>> > > R10: 0000000000000000 R11: 0000000000000000 R12: ffffffff85bd4020
>> > > R13: ffffffff84754ed2 R14: 0000000000000014 R15: ffff8801c4e26540
>> > > FS: 0000000000000000(0000) GS:ffff8801db300000(0000)
>> > > knlGS:0000000000000000
>> > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> > > CR2: 0000000000463610 CR3: 00000001c6698000 CR4: 00000000001406e0
>> > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> > > Call Trace:
>> > > <IRQ>
>> > > skb_over_panic net/core/skbuff.c:109 [inline]
>> > > skb_put+0x181/0x1c0 net/core/skbuff.c:1694
>> > > add_grhead.isra.24+0x42/0x3b0 net/ipv6/mcast.c:1695
>> > > add_grec+0xa55/0x1060 net/ipv6/mcast.c:1817
>> > > mld_send_cr net/ipv6/mcast.c:1903 [inline]
>> > > mld_ifc_timer_expire+0x4d2/0x770 net/ipv6/mcast.c:2448
>> > > call_timer_fn+0x23b/0x840 kernel/time/timer.c:1320
>> > > expire_timers kernel/time/timer.c:1357 [inline]
>> > > __run_timers+0x7e1/0xb60 kernel/time/timer.c:1660
>> > > run_timer_softirq+0x4c/0xb0 kernel/time/timer.c:1686
>> > > __do_softirq+0x29d/0xbb2 kernel/softirq.c:285
>> > > invoke_softirq kernel/softirq.c:365 [inline]
>> > > irq_exit+0x1d3/0x210 kernel/softirq.c:405
>> > > exiting_irq arch/x86/include/asm/apic.h:540 [inline]
>> > > smp_apic_timer_interrupt+0x16b/0x700
>> > > arch/x86/kernel/apic/apic.c:1052
>> > > apic_timer_interrupt+0xa9/0xb0 arch/x86/entry/entry_64.S:920
>> > > </IRQ>
>> > > RIP: 0010:native_safe_halt+0x6/0x10
>> > > arch/x86/include/asm/irqflags.h:54
>> > > RSP: 0018:ffff8801d9f97da8 EFLAGS: 00000282 ORIG_RAX:
>> > > ffffffffffffff11
>> > > RAX: dffffc0000000000 RBX: 1ffff1003b3f2fb8 RCX: 0000000000000000
>> > > RDX: 1ffffffff0c59734 RSI: 0000000000000001 RDI: ffffffff862cb9a0
>> > > RBP: ffff8801d9f97da8 R08: 0000000000000000 R09: 0000000000000000
>> > > R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
>> > > R13: ffff8801d9f97e60 R14: ffffffff869eb920 R15: 0000000000000000
>> > > arch_safe_halt arch/x86/include/asm/paravirt.h:93 [inline]
>> > > default_idle+0xbf/0x430 arch/x86/kernel/process.c:355
>> > > arch_cpu_idle+0xa/0x10 arch/x86/kernel/process.c:346
>> > > default_idle_call+0x36/0x90 kernel/sched/idle.c:98
>> > > cpuidle_idle_call kernel/sched/idle.c:156 [inline]
>> > > do_idle+0x24a/0x3b0 kernel/sched/idle.c:246
>> > > cpu_startup_entry+0x18/0x20 kernel/sched/idle.c:351
>> > > start_secondary+0x330/0x460 arch/x86/kernel/smpboot.c:277
>> > > secondary_startup_64+0xa5/0xb0 arch/x86/kernel/head_64.S:237
>> > > Code: 03 0f b6 04 01 84 c0 74 04 3c 03 7e 20 8b 4b 78 41 57 48 c7
>> > > c7 a0 38
>> > > bd 85 52 56 4c 89 ea 41 50 4c 89 e6 45 89 f0 e8 0c b6 3d fd <0f>
>> > > 0b 4c 89 4d
>> > > b8 4c 89 45 c0 48 89 75 c8 48 89 55 d0 e8 7d 93
>> > > RIP: skb_panic+0x15c/0x1f0 net/core/skbuff.c:100 RSP:
>> > > ffff8801db307508
>> > > ---[ end trace 941a8a0f633e271f ]---
>> > >
>> >
>> > This isn't a sctp problem, but mld's, seems when lo's mtu became 0,
>> > it allocs a skb without enough space in add_grec():
>> > if (AVAILABLE(skb) < sizeof(*psrc) +
>> > first*sizeof(struct mld2_grec)) {
>> > if (truncate && !first)
>> > break; /* truncate these */
>> > if (pgr)
>> > pgr->grec_nsrcs = htons(scount);
>> > if (skb)
>> > mld_sendpack(skb);
>> > skb = mld_newpack(idev, dev->mtu); <---
>> >
>> > I will check this for sure later on both igmp and mld.
>>
>> Fix:
>> --- a/net/ipv6/mcast.c
>> +++ b/net/ipv6/mcast.c
>> @@ -1766,8 +1766,8 @@ static struct sk_buff *add_grec(struct sk_buff
>> *skb, struct ifmcaddr6 *pmc,
>> if (isquery)
>> psf->sf_gsresp = 0;
>>
>> - if (AVAILABLE(skb) < sizeof(*psrc) +
>> - first*sizeof(struct mld2_grec)) {
>> + if (AVAILABLE(skb) < (int)(sizeof(*psrc) +
>> + first * sizeof(*pgr))) {
>> if (truncate && !first)
>> break; /* truncate these */
>> if (pgr)
>> @@ -1810,7 +1810,7 @@ static struct sk_buff *add_grec(struct sk_buff
>> *skb, struct ifmcaddr6 *pmc,
>> return skb;
>> if (pmc->mca_crcount || isquery || crsend) {
>> /* make sure we have room for group header */
>> - if (skb && AVAILABLE(skb) < sizeof(struct mld2_grec)) {
>> + if (skb && AVAILABLE(skb) < (int)sizeof(*pgr)) {
>> mld_sendpack(skb);
>> skb = NULL; /* add_grhead will get a new one */
>> }
>>
>> do the same on igmp.
>
> Thanks for the tentative patch.
>
> Quite a hack if you ask me.
>
> I would rather :
>
> 1) Read dev->mtu once to avoid bad assumptions/surprises.
>
> 2) Give up if this mtu is too small for IPV6 to be functional.
>
> Something like :
>
>
> net/ipv6/mcast.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
> index fc6d7d143f2c29aab9a3f56eae02e5337e65a97b..844642682b8363c4c32d329ed92474f834a59618 100644
> --- a/net/ipv6/mcast.c
> +++ b/net/ipv6/mcast.c
> @@ -1682,16 +1682,16 @@ static int grec_size(struct ifmcaddr6 *pmc, int type, int gdel, int sdel)
> }
>
> static struct sk_buff *add_grhead(struct sk_buff *skb, struct ifmcaddr6 *pmc,
> - int type, struct mld2_grec **ppgr)
> + int type, struct mld2_grec **ppgr, unsigned int mtu)
> {
> - struct net_device *dev = pmc->idev->dev;
> struct mld2_report *pmr;
> struct mld2_grec *pgr;
>
> - if (!skb)
> - skb = mld_newpack(pmc->idev, dev->mtu);
> - if (!skb)
> - return NULL;
> + if (!skb) {
> + skb = mld_newpack(pmc->idev, mtu);
> + if (!skb)
> + return NULL;
> + }
> pgr = skb_put(skb, sizeof(struct mld2_grec));
> pgr->grec_type = type;
> pgr->grec_auxwords = 0;
> @@ -1714,10 +1714,15 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
> struct mld2_grec *pgr = NULL;
> struct ip6_sf_list *psf, *psf_next, *psf_prev, **psf_list;
> int scount, stotal, first, isquery, truncate;
> + unsigned int mtu;
>
> if (pmc->mca_flags & MAF_NOREPORT)
> return skb;
>
> + mtu = READ_ONCE(dev->mtu);
> + if (mtu < IPV6_MIN_MTU)
> + return skb;
> +
> isquery = type == MLD2_MODE_IS_INCLUDE ||
> type == MLD2_MODE_IS_EXCLUDE;
> truncate = type == MLD2_MODE_IS_EXCLUDE ||
> @@ -1738,7 +1743,7 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
> AVAILABLE(skb) < grec_size(pmc, type, gdeleted, sdeleted)) {
> if (skb)
> mld_sendpack(skb);
> - skb = mld_newpack(idev, dev->mtu);
> + skb = mld_newpack(idev, mtu);
> }
> }
> first = 1;
> @@ -1774,12 +1779,12 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
> pgr->grec_nsrcs = htons(scount);
> if (skb)
> mld_sendpack(skb);
> - skb = mld_newpack(idev, dev->mtu);
> + skb = mld_newpack(idev, mtu);
> first = 1;
> scount = 0;
> }
> if (first) {
> - skb = add_grhead(skb, pmc, type, &pgr);
> + skb = add_grhead(skb, pmc, type, &pgr, mtu);
> first = 0;
> }
> if (!skb)
> @@ -1814,7 +1819,7 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
> mld_sendpack(skb);
> skb = NULL; /* add_grhead will get a new one */
> }
> - skb = add_grhead(skb, pmc, type, &pgr);
> + skb = add_grhead(skb, pmc, type, &pgr, mtu);
> }
> }
> if (pgr)
>
The new patch works to me, just two questions:
1. should it use "idev->cnf.mtu6" here for mld ?
2. 'if (int < unsigned int)' is still not nice, though in 'if
(AVAILABLE(skb) < sizeof())'
AVAILABLE(skb) seems always to return >= 0 after your patch.
^ permalink raw reply
* Re: kernel BUG at net/core/skbuff.c:LINE! (2)
From: Xin Long @ 2017-12-10 4:38 UTC (permalink / raw)
To: Cong Wang
Cc: syzbot, davem, kuznet, LKML, linux-sctp, network dev, Neil Horman,
syzkaller-bugs, Vlad Yasevich, yoshfuji
In-Reply-To: <CAM_iQpWffARgEqPTCnz=g0RDUUepJUQmfNF5A8DGa28sjcKo-w@mail.gmail.com>
On Sun, Dec 10, 2017 at 3:36 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Dec 8, 2017 at 12:45 AM, Xin Long <lucien.xin@gmail.com> wrote:
>> This isn't a sctp problem, but mld's, seems when lo's mtu became 0,
>> it allocs a skb without enough space in add_grec():
>
> Shouldn't we just set its min_mtu to ETH_MIN_MTU?
No idea why there's no min_mtu limitation for lo dev.
^ permalink raw reply
* Re: kernel BUG at net/core/skbuff.c:LINE! (2)
From: Eric Dumazet @ 2017-12-10 4:51 UTC (permalink / raw)
To: Xin Long
Cc: syzbot, davem, kuznet, LKML, linux-sctp, network dev, Neil Horman,
syzkaller-bugs, Vlad Yasevich, yoshfuji
In-Reply-To: <CADvbK_dMfCf82LYZWeWp1EOzStwuTyjP7SzbUwa0vnT5sc38Qw@mail.gmail.com>
On Sun, 2017-12-10 at 12:36 +0800, Xin Long wrote:
> The new patch works to me, just two questions:
> 1. should it use "idev->cnf.mtu6" here for mld ?
No idea why some parts of IPv6 would use a different view of device
mtu. Maybe others can comment.
>
> 2. 'if (int < unsigned int)' is still not nice, though in 'if
> (AVAILABLE(skb) < sizeof())'
> AVAILABLE(skb) seems always to return >= 0 after your patch.
Really if AVAILABLE(skb) was negative, a bug already happened.
^ permalink raw reply
* [RFC][PATCH] apparent big-endian bugs in dwc-xlgmac
From: Al Viro @ 2017-12-10 4:53 UTC (permalink / raw)
To: Jie Deng; +Cc: netdev
In xlgmac_dev_xmit():
/* Mark it as a CONTEXT descriptor */
dma_desc->desc3 = XLGMAC_SET_REG_BITS_LE(
dma_desc->desc3,
TX_CONTEXT_DESC3_CTXT_POS,
TX_CONTEXT_DESC3_CTXT_LEN,
1);
Looking at XLGMAC_SET_REG_BITS_LE() we see this:
#define XLGMAC_SET_REG_BITS_LE(var, pos, len, val) ({ \
typeof(var) _var = (var); \
typeof(pos) _pos = (pos); \
typeof(len) _len = (len); \
typeof(val) _val = (val); \
_val = (_val << _pos) & GENMASK(_pos + _len - 1, _pos); \
_var = (_var & ~GENMASK(_pos + _len - 1, _pos)) | _val; \
cpu_to_le32(_var); \
})
That thing assumes var to be host-endian and has a little-endian result.
Unfortunately, we feed it a little-endian and store the result back into
the same place. That might work if the original values *was* host-endian
and we wanted to end up with little-endian. However, that is immediately
followed by
/* Indicate this descriptor contains the MSS */
dma_desc->desc3 = XLGMAC_SET_REG_BITS_LE(
dma_desc->desc3,
TX_CONTEXT_DESC3_TCMSSV_POS,
TX_CONTEXT_DESC3_TCMSSV_LEN,
1);
where we operate on the now definitely little-endian value. That really
can't be right. I don't have the hardware in question, so I can't test
that, but it smells like this needs something like diff below, making
XLGMAC_SET_REG_BITS_LE take le32 and return le32. GET side of things
is le32 -> u32; definition looks correct, but slightly misannotated.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/drivers/net/ethernet/synopsys/dwc-xlgmac.h b/drivers/net/ethernet/synopsys/dwc-xlgmac.h
index cab3e40a86b9..e95c4c250e16 100644
--- a/drivers/net/ethernet/synopsys/dwc-xlgmac.h
+++ b/drivers/net/ethernet/synopsys/dwc-xlgmac.h
@@ -106,7 +106,7 @@
#define XLGMAC_GET_REG_BITS_LE(var, pos, len) ({ \
typeof(pos) _pos = (pos); \
typeof(len) _len = (len); \
- typeof(var) _var = le32_to_cpu((var)); \
+ u32 _var = le32_to_cpu((var)); \
((_var) & GENMASK(_pos + _len - 1, _pos)) >> (_pos); \
})
@@ -125,8 +125,8 @@
typeof(len) _len = (len); \
typeof(val) _val = (val); \
_val = (_val << _pos) & GENMASK(_pos + _len - 1, _pos); \
- _var = (_var & ~GENMASK(_pos + _len - 1, _pos)) | _val; \
- cpu_to_le32(_var); \
+ (_var & ~cpu_to_le32(GENMASK(_pos + _len - 1, _pos))) | \
+ cpu_to_le32(_val); \
})
struct xlgmac_pdata;
^ permalink raw reply related
* Re: kernel BUG at net/core/skbuff.c:LINE! (2)
From: Eric Dumazet @ 2017-12-10 5:12 UTC (permalink / raw)
To: Xin Long, Cong Wang
Cc: syzbot, davem, kuznet, LKML, linux-sctp, network dev, Neil Horman,
syzkaller-bugs, Vlad Yasevich, yoshfuji
In-Reply-To: <CADvbK_cAeiW-Tt0OnTBrzHObKOaNjRZnB1Uu+GSRJhhFpHTU2w@mail.gmail.com>
On Sun, 2017-12-10 at 12:38 +0800, Xin Long wrote:
> On Sun, Dec 10, 2017 at 3:36 AM, Cong Wang <xiyou.wangcong@gmail.com>
> wrote:
> > On Fri, Dec 8, 2017 at 12:45 AM, Xin Long <lucien.xin@gmail.com>
> > wrote:
> > > This isn't a sctp problem, but mld's, seems when lo's mtu became
> > > 0,
> > > it allocs a skb without enough space in add_grec():
> >
> > Shouldn't we just set its min_mtu to ETH_MIN_MTU?
>
> No idea why there's no min_mtu limitation for lo dev.
Because it wont solve this bug.
ETH_MIN_MTU is really about IPv4.
^ permalink raw reply
* Re: [PATCH net-next v3 1/5] net: Introduce NETIF_F_GRO_HW.
From: Michael Chan @ 2017-12-10 6:40 UTC (permalink / raw)
To: Alexander Duyck
Cc: David Miller, Netdev, Andrew Gospodarek, Ariel Elior,
everest-linux-l2
In-Reply-To: <CAKgT0UdKcVb+Qpmbdv=TfFTdnO9PGDgcTXpXf+eJ_y9k4wz_CA@mail.gmail.com>
On Sat, Dec 9, 2017 at 2:04 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Sat, Dec 9, 2017 at 1:31 PM, Michael Chan <michael.chan@broadcom.com> wrote:
>> On Sat, Dec 9, 2017 at 10:50 AM, Alexander Duyck
>> <alexander.duyck@gmail.com> wrote:
>>> So I would disagree with it being a subset of NETIF_F_GRO. If anything
>>> it is an alternative to NETIF_F_GRO. It is performing GRO much earlier
>>> at the device level in the case of hardware drivers. My concern is
>>> this is probably going to end up applying to things other than just
>>> hardware drivers though. For example what is to prevent this from
>>> being applied to something like a virtio/tap interface? It seems like
>>> this should be something that would be easy to implement in software.
>>
>> If you do it in software, it's called NETIF_F_GRO. We already have
>> it. The whole point of the new flag is that if the device has
>> software GRO enabled, and if the device supports GRO_HW, then we can
>> do a subset of GRO in hardware (hopefully faster).
>
> I can see what you are getting at. But GRO_HW with GRO stacked on top
> of it won't necessarily be the fastest form of GRO. If you have a
> GRO_HW implementation that is complete enough people may want to
> disable Software GRO in order to avoid the extra overhead involved
> with using it.
It is possible that if you have incoming packets 1, 2, 3, 4, 5 for a
TCP connection, HW_GRO can aggregate packets 1, 2, 3, but cannot
aggregate packets 4 and 5 due to hardware resource limitation.
Software GRO aggregates 4 and 5. So it works well together.
>>> I'm going to back off on my requirement for you to handle propagation
>>> since after spending a couple hours working on it I did find it was
>>> more complex then I originally thought it would be. With that said
>>> however I would want to see this feature implemented in such a way
>>> that we can deal with propagating the bits in the future if we need to
>>> and that is what I am basing my comments on.
>>
>> Nothing stops anyone from propagating the flag. Just add
>> NETIF_F_GRO_HW to NETIF_F_UPPER_DISABLES and it will be propagated
>> just like LRO.
>
> Yes, but the problem then is it doesn't solve the secondary issue of
> no way to propagate down the desire to disable GRO as well. That is
> why I am thinking that the new bit could be used to indicate that we
> want GRO to be supported either in the driver or below it instead of
> only in "hardware". We are much better off with a generic solution and
> that is why I think it might be better to use more of a pipeline or
> staged type definition for this. Basically with GRO it occurs in the
> GRO logic just after the driver hands off the packet, while this new
> bit indicates that GRO happens somewhere before then. If we use that
> definition for this then it becomes usable to deal with things like
> the stacked devices problem where the stacked devices normally have
> the GRO flag disabled since we don't want to run GRO multiple times,
> but as a result the stacked devices have no way of saying they don't
> want GRO. If we tweak the definition of this bit it solves that
> problem since it would allow for us disabling GRO, GRO_HW, and LRO on
> any devices below a given device.
I just don't follow your logic. First of all, GRO on an upper device
doesn't mean that we are doing GRO on the upper device. The bonding
driver cannot do GRO because it doesn't call napi_gro_receive(). GRO
always happens on the lower device. Propagation of GRO can only mean
that if GRO is set on the upper device, GRO is propagated and allowed
on lower devices. Nothing stops you from doing that if you want to do
that.
>>> I still disagree with this bit. I think GRO is a pure software
>>> offload, whereas GRO_HW can represent either a software offload of
>>> some sort occurring in or before the driver, or in the hardware.
>>> Basically the difference between the two as I view it is where the GRO
>>> is occurring. I would like to keep that distinction and make use of
>>> it. As I mentioned before in the case of bonding we currently have no
>>> way to disable GRO on the lower devices partially because GRO is a
>>> pure software feature and always happens at each device along the way.
>>> The nice thing about this new bit is the assumption is that it is
>>> pushing GRO to the lowest possible level and not triggering any side
>>> effects like GRO currently does. I hope to use that logic with stacked
>>> devices so that we could clear the bit and have it disable GRO,
>>> GRO_HW, and LRO on all devices below the device that cleared it.
>>>
>>> I think this linking of GRO and GRO_HW is something that would be
>>> better served by moving it into the driver if you are wanting to
>>> maintain the behavior of how this was previously linked to GRO.
>>
>> If you insist, I can move this to the driver's ndo_fix_features().
>> But I feel it is much better to enforce this dependency system wide.
>> Once again, GRO_HW is hardware accelerated GRO and should depend on
>> it.
>
> The question I would have is why? Where is the dependency? I don't see
> it. It is GRO in one spot and/or GRO in the other. The two don't
> interract directly and I don't believe you can do software GRO on a
> frame that has already been coalesced in hardware,
Right. But hardware can do a series of frames and software can do a
different series of frames that have not been aggregated.
>> This is a logical feature dependency that Yuval Mintz suggested. For
>> GRO_HW to work, hardware must verify the checksum of a packet before
>> the packet can be merged.
>>
>> So if the user does not want to do RXCSUM on this device for whatever
>> reason, it logically means that he also doesn't want to do GRO_HW with
>> implied RXCSUM performed on each packet that is merged.
>>
>> So I agree with Yuval that this dependency makes sense.
>
> Okay then, so if we are going to go that route we may want to be
> complete on this and just disable GRO_HW and LRO if RXCSUM is not
> enabled. We might also want to add a comment indicating that we don't
> support anything that might mangle a packet at the driver level if
> RXCSUM is not enabled. Comments explaining all this would be a good
> thing just to keep someone from grabbing GRO and lumping it in at some
> point in the future.
>
> I'm still working on trying to propagate the Rx checksum properly
> since it should probably follow the same UPPER_DISABLES behavior as
> LRO, but I will probably only have a few hours over the next week to
> really work on any code and there end up being a number of stacked
> drivers that have to be updated. I would be good with just flipping
> this logic for now and if RXCSUM is not set, and GRO_HW (just noticed
> the typo in your message) is set, then print your message and clear
> the bit. I can probably come back later and add LRO once I get the
> propagation bits worked out.
Just fix the netdev_dbg() typo, right? I don't understand what you
mean by flipping the logic. It's the same whether you check RXCSUM
first or GRO_HW first.
May be you meant put the RXCSUM check in the outer if statement so
that someone could add more inner checks? OK, I think that's what you
meant.
>
> As far as patch 2 in the set it would probably be better to either
> drop it and just accept it as an outstanding issue, or you could take
> on the propagation problems with GRO_HW and RXCSUM since we really
> need to get those solved in order for this functionality to fully
> work.
We need patch #2 otherwise generic GRO won't work on these 3 drivers.
I don't think I fully understand your concerns about propagation. To
me propagation is just a usage model where an upper device will
control the common features of lower devices. It is more convenient
to have propagation, but requires upper devices to be aware of all
features that propagate (GRO, RXCSUM). Without propagation, it is
still fine.
^ permalink raw reply
* Re: [PATCH net-next v3 2/5] net: Disable GRO_HW when generic XDP is installed on a device.
From: Michael Chan @ 2017-12-10 6:49 UTC (permalink / raw)
To: Alexander Duyck
Cc: David Miller, Netdev, Andrew Gospodarek, Ariel Elior,
everest-linux-l2
In-Reply-To: <CAKgT0UeqWjHEFYLzbSk5O_7xV=QYYtPd-HdnRaGgpYy=d3yhLg@mail.gmail.com>
On Sat, Dec 9, 2017 at 2:37 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Sat, Dec 9, 2017 at 1:40 PM, Michael Chan <michael.chan@broadcom.com> wrote:
>> On Sat, Dec 9, 2017 at 10:56 AM, Alexander Duyck
>> <alexander.duyck@gmail.com> wrote:
>>> I think these two lines are redundant in dev_disable_lro, since
>>> netdev_update_features should propagate the disable to all of the
>>> lower devices.
>>
>> Right. But for GRO_HW, there is no automatic propagation.
>
> Right, but that is also an issue since the automatic propagation is
> what prevents LRO from being re-enabled on the lower devices.
>
>>> Also this doesn't prevent the lower devices from
>>> re-enabling gro_hw.
>>
>> Right. You can re-enable LRO on the upper device as well.
>
> On the upper device yes, but not on the lower devices. That was what I
> was getting at. With LRO there is netdev_sync_upper_features() and
> that prevents you from enabling it if the upper device has it
> disabled. The problem is right now there is nothing that sets it for
> the upper devices when they are added to something like a bond so that
> is one of the pieces that still has to be worked on before we can just
> use the existing sync logic.
I understand. But if the user really wants to re-enable LRO, he can
just re-enable LRO on the upper device first and then re-enable LRO on
the lower devices.
To permanently disable a feature, I think additional infrastructure
may be required so that the feature can be cleared in hw_features and
then re-enabled later when it is allowed.
^ permalink raw reply
* [PATCH net] sctp: make sure stream nums can match optlen in sctp_setsockopt_reset_streams
From: Xin Long @ 2017-12-10 7:40 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Neil Horman, syzkaller
Now in sctp_setsockopt_reset_streams, it only does the check
optlen < sizeof(*params) for optlen. But it's not enough, as
params->srs_number_streams should also match optlen.
If the streams in params->srs_stream_list are less than stream
nums in params->srs_number_streams, later when dereferencing
the stream list, it could cause a slab-out-of-bounds crash, as
reported by syzbot.
This patch is to fix it by also checking the stream numbers in
sctp_setsockopt_reset_streams to make sure at least it's not
greater than the streams in the list.
Fixes: 7f9d68ac944e ("sctp: implement sender-side procedures for SSN Reset Request Parameter")
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/socket.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 014847e..dbf140d 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3891,13 +3891,17 @@ static int sctp_setsockopt_reset_streams(struct sock *sk,
struct sctp_association *asoc;
int retval = -EINVAL;
- if (optlen < sizeof(struct sctp_reset_streams))
+ if (optlen < sizeof(*params))
return -EINVAL;
params = memdup_user(optval, optlen);
if (IS_ERR(params))
return PTR_ERR(params);
+ if (params->srs_number_streams * sizeof(__u16) >
+ optlen - sizeof(*params))
+ goto out;
+
asoc = sctp_id2assoc(sk, params->srs_assoc_id);
if (!asoc)
goto out;
--
2.1.0
^ permalink raw reply related
* v4.15-rc2 on thinkpad x60: ethernet stopped working
From: Pavel Machek @ 2017-12-10 8:39 UTC (permalink / raw)
To: kernel list; +Cc: jeffrey.t.kirsher, intel-wired-lan, netdev
[-- Attachment #1: Type: text/plain, Size: 899 bytes --]
Hi!
In v4.15-rc2+, network manager can not see my ethernet card, and
manual attempts to ifconfig it up did not really help, either.
Card is:
02:00.0 Ethernet controller: Intel Corporation 82573L Gigabit Ethernet
Controller
Dmesg says:
dmesg | grep eth
[ 0.648931] e1000e 0000:02:00.0 eth0: (PCI Express:2.5GT/s:Width
x1) 00:16:d3:25:19:04
[ 0.648934] e1000e 0000:02:00.0 eth0: Intel(R) PRO/1000 Network
Connection
[ 0.649012] e1000e 0000:02:00.0 eth0: MAC: 2, PHY: 2, PBA No:
005302-003
[ 0.706510] usbcore: registered new interface driver cdc_ether
[ 6.557022] e1000e 0000:02:00.0 eth1: renamed from eth0
[ 6.577554] systemd-udevd[2363]: renamed network interface eth0 to
eth1
Any ideas?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* [PATCH net] fou: fix some member types in guehdr
From: Xin Long @ 2017-12-10 8:56 UTC (permalink / raw)
To: network dev; +Cc: davem, Tom Herbert
guehdr struct is used to build or parse gue packets, which
are always in big endian. It's better to define all guehdr
members as __beXX types.
Also, in validate_gue_flags it's not good to use a __be32
variable for both Standard flags(__be16) and Private flags
(__be32), and pass it to other funcions.
This patch could fix a bunch of sparse warnings from fou.
Fixes: 5024c33ac354 ("gue: Add infrastructure for flags and options")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/gue.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/net/gue.h b/include/net/gue.h
index 2fdb29c..fdad414 100644
--- a/include/net/gue.h
+++ b/include/net/gue.h
@@ -44,10 +44,10 @@ struct guehdr {
#else
#error "Please fix <asm/byteorder.h>"
#endif
- __u8 proto_ctype;
- __u16 flags;
+ __u8 proto_ctype;
+ __be16 flags;
};
- __u32 word;
+ __be32 word;
};
};
@@ -84,11 +84,10 @@ static inline size_t guehdr_priv_flags_len(__be32 flags)
* if there is an unknown standard or private flags, or the options length for
* the flags exceeds the options length specific in hlen of the GUE header.
*/
-static inline int validate_gue_flags(struct guehdr *guehdr,
- size_t optlen)
+static inline int validate_gue_flags(struct guehdr *guehdr, size_t optlen)
{
+ __be16 flags = guehdr->flags;
size_t len;
- __be32 flags = guehdr->flags;
if (flags & ~GUE_FLAGS_ALL)
return 1;
@@ -101,12 +100,13 @@ static inline int validate_gue_flags(struct guehdr *guehdr,
/* Private flags are last four bytes accounted in
* guehdr_flags_len
*/
- flags = *(__be32 *)((void *)&guehdr[1] + len - GUE_LEN_PRIV);
+ __be32 pflags = *(__be32 *)((void *)&guehdr[1] +
+ len - GUE_LEN_PRIV);
- if (flags & ~GUE_PFLAGS_ALL)
+ if (pflags & ~GUE_PFLAGS_ALL)
return 1;
- len += guehdr_priv_flags_len(flags);
+ len += guehdr_priv_flags_len(pflags);
if (len > optlen)
return 1;
}
--
2.1.0
^ permalink raw reply related
* Re: [PATCH 1/2] net: sh_eth: add support for SH7786
From: Sergei Shtylyov @ 2017-12-10 11:55 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: David S. Miller, Niklas Söderlund, Geert Uytterhoeven,
Simon Horman, netdev, linux-renesas-soc
In-Reply-To: <20171208164017.3e9a7cc6@windsurf.lan>
Hello!
On 12/08/2017 06:40 PM, Thomas Petazzoni wrote:
>>>>>> This commit adds the sh_eth_cpu_data structure that describes the
>>>>>> SH7786 variant of the IP.
>>>>>
>>>>> The manual seems to be unavailable, so I have to trust you. :-)
>>>>
>>>> Yes, sadly. However, if you tell me what to double check, I'd be happy
>>>> to do so.
>>>
>>> I have the manual now, will check against it...
>>> DaveM, I'm retracting my ACK for the time being.
>>
>> Starting to look into the manual, the current patch is wrong. SH7786 SoC
>> was probably the 1st one to use what we thought was R-Car specific register
>> layout. Definite NAK on this version.
>
> Thanks for the feedback. How do we proceed from there ? I don't have
Please use SH_ETH_REG_FAST_RCAR for the register layout.
> access to a lot of datasheets of the different Renesas SoCs, so it's
> not easy to figure out which IP variant the SH7786 is using compared to
> other Renesas SoCs.
I've already done that for you. :-)
> Just out of curiosity, which specific aspect makes you think the
> proposed patch is wrong ?
Total Ether register/bit documentation rehaul done for SH7786/R-Car --
including the register (and bit) rename and moving the registers to different
offsets...
> Have you noticed a specific register or field
> that isn't compatible with SH_ETH_REG_FAST_SH4 layout ?
There are surely SH4 registers that don't exist on SH7786 -- like BCFRR,
RTRATE, RPADIR, RBWAR, RDFAR, TBRAR, TDFAR...
> Note that my patch makes Ethernet work in practice on SH7784, I have
> root over NFS working as we speak.
I don't doubt it...
> This certainly doesn't mean that the
> patch is entirely correct, but it definitely means that the
> SH_ETH_REG_FAST_SH4 is close enough to what the SH7786 is using :-)
SH_ETH_REG_FAST_RCAR is definitely closer. :-)
> Thanks!
>
> Thomas
MBR, Sergei
^ permalink raw reply
* Module compile error
From: Алексей Болдырев @ 2017-12-10 11:58 UTC (permalink / raw)
To: netdev
CC [M] drivers/net/ethernet/intel/ixgbe/ixgbe_main.o
In file included from ./include/net/vxlan.h:6:0,
from drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:60:
./include/net/dst_metadata.h: In function ‘skb_vpls_info’:
./include/net/dst_metadata.h:36:9: error: implicit declaration of function ‘skb_metadata_dst’ [-Werror=implicit-function-declaration]
struct metadata_dst *md_dst = skb_metadata_dst(skb);
^
./include/net/dst_metadata.h:36:32: warning: initialization makes pointer from integer without a cast
struct metadata_dst *md_dst = skb_metadata_dst(skb);
^
./include/net/dst_metadata.h: At top level:
./include/net/dst_metadata.h:42:36: error: conflicting types for ‘skb_metadata_dst’
static inline struct metadata_dst *skb_metadata_dst(struct sk_buff *skb)
^
./include/net/dst_metadata.h:36:32: note: previous implicit declaration of ‘skb_metadata_dst’ was here
struct metadata_dst *md_dst = skb_metadata_dst(skb);
^
cc1: some warnings being treated as errors
scripts/Makefile.build:302: recipe for target 'drivers/net/ethernet/intel/ixgbe/ixgbe_main.o' failed
make[5]: *** [drivers/net/ethernet/intel/ixgbe/ixgbe_main.o] Error 1
scripts/Makefile.build:561: recipe for target 'drivers/net/ethernet/intel/ixgbe' failed
make[4]: *** [drivers/net/ethernet/intel/ixgbe] Error 2
scripts/Makefile.build:561: recipe for target 'drivers/net/ethernet/intel' failed
make[3]: *** [drivers/net/ethernet/intel] Error 2
scripts/Makefile.build:561: recipe for target 'drivers/net/ethernet' failed
make[2]: *** [drivers/net/ethernet] Error 2
scripts/Makefile.build:561: recipe for target 'drivers/net' failed
make[1]: *** [drivers/net] Error 2
Makefile:1019: recipe for target 'drivers' failed
make: *** [drivers] Error 2
This patch is https://github.com/eqvinox/vpls-linux-kernel/commit/d9d8efec0760bcea54496e7a3ed45c72316ffb6b#diff-00c78c934b7e227c38ff1b1b571db9e6
What could be the problem?
Kernel: 4.13.16
^ permalink raw reply
* Re: [PATCH 1/2] net: sh_eth: add support for SH7786
From: Sergei Shtylyov @ 2017-12-10 12:20 UTC (permalink / raw)
To: Thomas Petazzoni, David S. Miller, Niklas Söderlund,
Geert Uytterhoeven, Simon Horman
Cc: netdev, linux-renesas-soc
In-Reply-To: <20171204141744.18599-2-thomas.petazzoni@free-electrons.com>
On 12/04/2017 05:17 PM, Thomas Petazzoni wrote:
> This commit adds the sh_eth_cpu_data structure that describes the
> SH7786 variant of the IP.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> drivers/net/ethernet/renesas/sh_eth.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 0074c5998481..a3c48b2a713c 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -710,6 +710,30 @@ static struct sh_eth_cpu_data sh7724_data = {
> .rpadir_value = 0x00020000, /* NET_IP_ALIGN assumed to be 2 */
> };
>
> +static struct sh_eth_cpu_data sh7786_data = {
> + .set_duplex = sh_eth_set_duplex,
Hm, no bitrate switching?
(ECMR.OLB is said to be unused indeed in the manual...)
> +
> + .register_type = SH_ETH_REG_FAST_SH4,
SH_ETH_REG_FAST_RCAR.
> +
> + .ecsr_value = ECSR_PSRTO | ECSR_LCHNG | ECSR_ICD,
> + .ecsipr_value = ECSIPR_PSRTOIP | ECSIPR_LCHNGIP | ECSIPR_ICDIP,
Good enough (though magic packet interrupt should work too)...
> + .eesipr_value = EESIPR_RFCOFIP | EESIPR_ADEIP | EESIPR_ECIIP |
> + EESIPR_FTCIP | EESIPR_TDEIP | EESIPR_TFUFIP |
> + EESIPR_FRIP | EESIPR_RDEIP | EESIPR_RFOFIP |
> + EESIPR_RMAFIP | EESIPR_RRFIP |
> + EESIPR_RTLFIP | EESIPR_RTSFIP |
> + EESIPR_PREIP | EESIPR_CERFIP,
> +
> + .tx_check = EESR_FTC | EESR_CND | EESR_DLC | EESR_CD | EESR_RTO,
> + .eesr_err_check = EESR_TWB | EESR_TABT | EESR_RABT | EESR_RFE |
> + EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE,
I think you also need:
.fdr_value = 0x00000f0f,
like on R-Car gen1 SoCs...
[...]
The reset looks good...
MBR, Sergei
^ permalink raw reply
* Re: [PATCH 1/2] net: sh_eth: add support for SH7786
From: Sergei Shtylyov @ 2017-12-10 12:41 UTC (permalink / raw)
To: Thomas Petazzoni, David S. Miller, Niklas Söderlund,
Geert Uytterhoeven, Simon Horman
Cc: netdev, linux-renesas-soc
In-Reply-To: <86b47029-6474-31c9-b48a-9f5215b73378@cogentembedded.com>
On 12/10/2017 03:20 PM, Sergei Shtylyov wrote:
[...]
> The reset looks good...
Sorry, I meant to type "rest". :-)
MBR, Sergei
^ permalink raw reply
* Re: v4.15-rc2 on thinkpad x60: ethernet stopped working
From: Gabriel C @ 2017-12-10 12:44 UTC (permalink / raw)
To: Pavel Machek, kernel list; +Cc: jeffrey.t.kirsher, intel-wired-lan, netdev
In-Reply-To: <20171210083949.GA3872@amd>
On 10.12.2017 09:39, Pavel Machek wrote:
> Hi!
Hi,
> In v4.15-rc2+, network manager can not see my ethernet card, and
> manual attempts to ifconfig it up did not really help, either.
>
> Card is:
>
> 02:00.0 Ethernet controller: Intel Corporation 82573L Gigabit Ethernet
> Controller
>
> Dmesg says:
>
> dmesg | grep eth
> [ 0.648931] e1000e 0000:02:00.0 eth0: (PCI Express:2.5GT/s:Width
> x1) 00:16:d3:25:19:04
> [ 0.648934] e1000e 0000:02:00.0 eth0: Intel(R) PRO/1000 Network
> Connection
> [ 0.649012] e1000e 0000:02:00.0 eth0: MAC: 2, PHY: 2, PBA No:
> 005302-003
> [ 0.706510] usbcore: registered new interface driver cdc_ether
> [ 6.557022] e1000e 0000:02:00.0 eth1: renamed from eth0
> [ 6.577554] systemd-udevd[2363]: renamed network interface eth0 to
> eth1
>
> Any ideas ?
Yes , 19110cfbb34d4af0cdfe14cd243f3b09dc95b013 broke it.
See:
https://bugzilla.kernel.org/show_bug.cgi?id=198047
Fix there :
https://marc.info/?l=linux-kernel&m=151272209903675&w=2
Regards,
Gabriel C
^ permalink raw reply
* Re: [PATCH 1/2] net: sh_eth: add support for SH7786
From: Sergei Shtylyov @ 2017-12-10 12:46 UTC (permalink / raw)
To: Thomas Petazzoni, David S. Miller, Niklas Söderlund,
Geert Uytterhoeven, Simon Horman
Cc: netdev, linux-renesas-soc
In-Reply-To: <86b47029-6474-31c9-b48a-9f5215b73378@cogentembedded.com>
On 12/10/2017 03:20 PM, Sergei Shtylyov wrote:
>> This commit adds the sh_eth_cpu_data structure that describes the
>> SH7786 variant of the IP.
>>
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> ---
>> drivers/net/ethernet/renesas/sh_eth.c | 25 +++++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c
>> b/drivers/net/ethernet/renesas/sh_eth.c
>> index 0074c5998481..a3c48b2a713c 100644
>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>> @@ -710,6 +710,30 @@ static struct sh_eth_cpu_data sh7724_data = {
>> .rpadir_value = 0x00020000, /* NET_IP_ALIGN assumed to be 2 */
>> };
>> +static struct sh_eth_cpu_data sh7786_data = {
>> + .set_duplex = sh_eth_set_duplex,
>
> Hm, no bitrate switching?
> (ECMR.OLB is said to be unused indeed in the manual...)
I meant CXR20.OLB -- as it's called in the SH7786 and R-Car manuals.
MBR, Sergei
^ permalink raw reply
* Re: [PATCH net] sctp: make sure stream nums can match optlen in sctp_setsockopt_reset_streams
From: Marcelo Ricardo Leitner @ 2017-12-10 13:51 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman, syzkaller
In-Reply-To: <a6c86b9cb13c2b6e2ed2539c028557a6b1a713ef.1512891651.git.lucien.xin@gmail.com>
On Sun, Dec 10, 2017 at 03:40:51PM +0800, Xin Long wrote:
> Now in sctp_setsockopt_reset_streams, it only does the check
> optlen < sizeof(*params) for optlen. But it's not enough, as
> params->srs_number_streams should also match optlen.
>
> If the streams in params->srs_stream_list are less than stream
> nums in params->srs_number_streams, later when dereferencing
> the stream list, it could cause a slab-out-of-bounds crash, as
> reported by syzbot.
>
> This patch is to fix it by also checking the stream numbers in
> sctp_setsockopt_reset_streams to make sure at least it's not
> greater than the streams in the list.
>
> Fixes: 7f9d68ac944e ("sctp: implement sender-side procedures for SSN Reset Request Parameter")
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> net/sctp/socket.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 014847e..dbf140d 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -3891,13 +3891,17 @@ static int sctp_setsockopt_reset_streams(struct sock *sk,
> struct sctp_association *asoc;
> int retval = -EINVAL;
>
> - if (optlen < sizeof(struct sctp_reset_streams))
> + if (optlen < sizeof(*params))
> return -EINVAL;
>
> params = memdup_user(optval, optlen);
> if (IS_ERR(params))
> return PTR_ERR(params);
>
> + if (params->srs_number_streams * sizeof(__u16) >
> + optlen - sizeof(*params))
> + goto out;
> +
> asoc = sctp_id2assoc(sk, params->srs_assoc_id);
> if (!asoc)
> goto out;
> --
> 2.1.0
>
^ permalink raw reply
* [PATCH v6 1/3] sock: Change the netns_core member name.
From: Tonghao Zhang @ 2017-12-10 15:12 UTC (permalink / raw)
To: davem, xiyou.wangcong, edumazet, willemb, xemul; +Cc: netdev, Tonghao Zhang
Change the member name will make the code more readable.
This patch will be used in next patch.
Signed-off-by: Martin Zhang <zhangjunweimartin@didichuxing.com>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
include/net/netns/core.h | 2 +-
net/core/sock.c | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/net/netns/core.h b/include/net/netns/core.h
index 0ad4d0c..45cfb5d 100644
--- a/include/net/netns/core.h
+++ b/include/net/netns/core.h
@@ -11,7 +11,7 @@ struct netns_core {
int sysctl_somaxconn;
- struct prot_inuse __percpu *inuse;
+ struct prot_inuse __percpu *prot_inuse;
};
#endif
diff --git a/net/core/sock.c b/net/core/sock.c
index c0b5b2f..c2dd2d3 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -3045,7 +3045,7 @@ struct prot_inuse {
void sock_prot_inuse_add(struct net *net, struct proto *prot, int val)
{
- __this_cpu_add(net->core.inuse->val[prot->inuse_idx], val);
+ __this_cpu_add(net->core.prot_inuse->val[prot->inuse_idx], val);
}
EXPORT_SYMBOL_GPL(sock_prot_inuse_add);
@@ -3055,7 +3055,7 @@ int sock_prot_inuse_get(struct net *net, struct proto *prot)
int res = 0;
for_each_possible_cpu(cpu)
- res += per_cpu_ptr(net->core.inuse, cpu)->val[idx];
+ res += per_cpu_ptr(net->core.prot_inuse, cpu)->val[idx];
return res >= 0 ? res : 0;
}
@@ -3063,13 +3063,13 @@ int sock_prot_inuse_get(struct net *net, struct proto *prot)
static int __net_init sock_inuse_init_net(struct net *net)
{
- net->core.inuse = alloc_percpu(struct prot_inuse);
- return net->core.inuse ? 0 : -ENOMEM;
+ net->core.prot_inuse = alloc_percpu(struct prot_inuse);
+ return net->core.prot_inuse ? 0 : -ENOMEM;
}
static void __net_exit sock_inuse_exit_net(struct net *net)
{
- free_percpu(net->core.inuse);
+ free_percpu(net->core.prot_inuse);
}
static struct pernet_operations net_inuse_ops = {
--
1.8.3.1
^ permalink raw reply related
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