* 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
* 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: 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: [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
* [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
* [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
* 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
* 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] 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: 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
* [PATCH 3/3] r8169: remove netif_napi_del in probe error path
From: Heiner Kallweit @ 2017-12-09 23:43 UTC (permalink / raw)
To: Realtek linux nic maintainers, Bjorn Helgaas
Cc: netdev@vger.kernel.org, linux-pci@vger.kernel.org
In-Reply-To: <9eee5aba-a739-a609-fd65-76666f377ed1@gmail.com>
netif_napi_del is called implicitely by free_netdev, therefore we
don't have to do it explicitely.
When the probe error path is reached, the net_device isn't
registered yet. Therefore reordering the call to netif_napi_del
shouldn't cause any issues.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 3c7d90d3a..857f67beb 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -8672,14 +8672,12 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
tp->counters = dmam_alloc_coherent (&pdev->dev, sizeof(*tp->counters),
&tp->counters_phys_addr,
GFP_KERNEL);
- if (!tp->counters) {
- rc = -ENOMEM;
- goto err_out_msi_5;
- }
+ if (!tp->counters)
+ return -ENOMEM;
rc = register_netdev(dev);
if (rc < 0)
- goto err_out_msi_5;
+ return rc;
pci_set_drvdata(pdev, dev);
@@ -8709,11 +8707,6 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
netif_carrier_off(dev);
return 0;
-
-err_out_msi_5:
- netif_napi_del(&tp->napi);
-
- return rc;
}
static struct pci_driver rtl8169_pci_driver = {
--
2.15.1
^ permalink raw reply related
* [PATCH 2/3] r8169: switch to device-managed functions in probe
From: Heiner Kallweit @ 2017-12-09 23:43 UTC (permalink / raw)
To: Realtek linux nic maintainers, Bjorn Helgaas
Cc: netdev@vger.kernel.org, linux-pci@vger.kernel.org
In-Reply-To: <9eee5aba-a739-a609-fd65-76666f377ed1@gmail.com>
Simplify probe error path and remove callback by using device-managed
functions.
rtl_disable_msi isn't needed any longer because the release callback
of pcim_enable_device does this implicitely.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 80 +++++++++---------------------------
1 file changed, 20 insertions(+), 60 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index fc0d5fa65..3c7d90d3a 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -4643,16 +4643,6 @@ static void rtl8169_phy_timer(struct timer_list *t)
rtl_schedule_task(tp, RTL_FLAG_TASK_PHY_PENDING);
}
-static void rtl8169_release_board(struct pci_dev *pdev, struct net_device *dev,
- void __iomem *ioaddr)
-{
- iounmap(ioaddr);
- pci_release_regions(pdev);
- pci_clear_mwi(pdev);
- pci_disable_device(pdev);
- free_netdev(dev);
-}
-
DECLARE_RTL_COND(rtl_phy_reset_cond)
{
return tp->phy_reset_pending(tp);
@@ -4784,14 +4774,6 @@ static int rtl_tbi_ioctl(struct rtl8169_private *tp, struct mii_ioctl_data *data
return -EOPNOTSUPP;
}
-static void rtl_disable_msi(struct pci_dev *pdev, struct rtl8169_private *tp)
-{
- if (tp->features & RTL_FEATURE_MSI) {
- pci_disable_msi(pdev);
- tp->features &= ~RTL_FEATURE_MSI;
- }
-}
-
static void rtl_init_mdio_ops(struct rtl8169_private *tp)
{
struct mdio_ops *ops = &tp->mdio_ops;
@@ -8256,9 +8238,6 @@ static void rtl_remove_one(struct pci_dev *pdev)
unregister_netdev(dev);
- dma_free_coherent(&tp->pci_dev->dev, sizeof(*tp->counters),
- tp->counters, tp->counters_phys_addr);
-
rtl_release_firmware(tp);
if (pci_dev_run_wake(pdev))
@@ -8266,9 +8245,6 @@ static void rtl_remove_one(struct pci_dev *pdev)
/* restore original MAC address */
rtl_rar_set(tp, dev->perm_addr);
-
- rtl_disable_msi(pdev, tp);
- rtl8169_release_board(pdev, dev, tp->mmio_addr);
}
static const struct net_device_ops rtl_netdev_ops = {
@@ -8445,11 +8421,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
MODULENAME, RTL8169_VERSION);
}
- dev = alloc_etherdev(sizeof (*tp));
- if (!dev) {
- rc = -ENOMEM;
- goto out;
- }
+ dev = devm_alloc_etherdev(&pdev->dev, sizeof (*tp));
+ if (!dev)
+ return -ENOMEM;
SET_NETDEV_DEV(dev, &pdev->dev);
dev->netdev_ops = &rtl_netdev_ops;
@@ -8472,13 +8446,13 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
PCIE_LINK_STATE_CLKPM);
/* enable device (incl. PCI PM wakeup and hotplug setup) */
- rc = pci_enable_device(pdev);
+ rc = pcim_enable_device(pdev);
if (rc < 0) {
netif_err(tp, probe, dev, "enable failure\n");
- goto err_out_free_dev_1;
+ return rc;
}
- if (pci_set_mwi(pdev) < 0)
+ if (pcim_set_mwi(pdev) < 0)
netif_info(tp, probe, dev, "Mem-Wr-Inval unavailable\n");
/* make sure PCI base addr 1 is MMIO */
@@ -8486,30 +8460,28 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
netif_err(tp, probe, dev,
"region #%d not an MMIO resource, aborting\n",
region);
- rc = -ENODEV;
- goto err_out_mwi_2;
+ return -ENODEV;
}
/* check for weird/broken PCI region reporting */
if (pci_resource_len(pdev, region) < R8169_REGS_SIZE) {
netif_err(tp, probe, dev,
"Invalid PCI region size(s), aborting\n");
- rc = -ENODEV;
- goto err_out_mwi_2;
+ return -ENODEV;
}
rc = pci_request_regions(pdev, MODULENAME);
if (rc < 0) {
netif_err(tp, probe, dev, "could not request regions\n");
- goto err_out_mwi_2;
+ return rc;
}
/* ioremap MMIO region */
- ioaddr = ioremap(pci_resource_start(pdev, region), R8169_REGS_SIZE);
+ ioaddr = devm_ioremap(&pdev->dev, pci_resource_start(pdev, region),
+ R8169_REGS_SIZE);
if (!ioaddr) {
netif_err(tp, probe, dev, "cannot remap MMIO, aborting\n");
- rc = -EIO;
- goto err_out_free_res_3;
+ return -EIO;
}
tp->mmio_addr = ioaddr;
@@ -8535,7 +8507,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
if (rc < 0) {
netif_err(tp, probe, dev, "DMA configuration failed\n");
- goto err_out_unmap_4;
+ return rc;
}
}
@@ -8697,8 +8669,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
tp->rtl_fw = RTL_FIRMWARE_UNKNOWN;
- tp->counters = dma_alloc_coherent (&pdev->dev, sizeof(*tp->counters),
- &tp->counters_phys_addr, GFP_KERNEL);
+ tp->counters = dmam_alloc_coherent (&pdev->dev, sizeof(*tp->counters),
+ &tp->counters_phys_addr,
+ GFP_KERNEL);
if (!tp->counters) {
rc = -ENOMEM;
goto err_out_msi_5;
@@ -8706,7 +8679,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
rc = register_netdev(dev);
if (rc < 0)
- goto err_out_cnt_6;
+ goto err_out_msi_5;
pci_set_drvdata(pdev, dev);
@@ -8735,25 +8708,12 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
netif_carrier_off(dev);
-out:
- return rc;
+ return 0;
-err_out_cnt_6:
- dma_free_coherent(&pdev->dev, sizeof(*tp->counters), tp->counters,
- tp->counters_phys_addr);
err_out_msi_5:
netif_napi_del(&tp->napi);
- rtl_disable_msi(pdev, tp);
-err_out_unmap_4:
- iounmap(ioaddr);
-err_out_free_res_3:
- pci_release_regions(pdev);
-err_out_mwi_2:
- pci_clear_mwi(pdev);
- pci_disable_device(pdev);
-err_out_free_dev_1:
- free_netdev(dev);
- goto out;
+
+ return rc;
}
static struct pci_driver rtl8169_pci_driver = {
--
2.15.1
^ permalink raw reply related
* [PATCH 1/3] PCI: introduce a device-managed version of pci_set_mwi
From: Heiner Kallweit @ 2017-12-09 23:43 UTC (permalink / raw)
To: Realtek linux nic maintainers, Bjorn Helgaas
Cc: netdev@vger.kernel.org, linux-pci@vger.kernel.org
In-Reply-To: <9eee5aba-a739-a609-fd65-76666f377ed1@gmail.com>
Introduce a device-managed version of pci_set_mwi. First user is the
Realtek r8169 driver.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/pci/pci.c | 29 +++++++++++++++++++++++++++++
include/linux/pci.h | 1 +
2 files changed, 30 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 4a7c6864f..fc57c378d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1458,6 +1458,7 @@ struct pci_devres {
unsigned int pinned:1;
unsigned int orig_intx:1;
unsigned int restore_intx:1;
+ unsigned int mwi:1;
u32 region_mask;
};
@@ -1476,6 +1477,9 @@ static void pcim_release(struct device *gendev, void *res)
if (this->region_mask & (1 << i))
pci_release_region(dev, i);
+ if (this->mwi)
+ pci_clear_mwi(dev);
+
if (this->restore_intx)
pci_intx(dev, this->orig_intx);
@@ -3760,6 +3764,31 @@ int pci_set_mwi(struct pci_dev *dev)
}
EXPORT_SYMBOL(pci_set_mwi);
+/**
+ * pcim_set_mwi - Managed pci_set_mwi()
+ * @dev: the PCI device for which MWI is enabled
+ *
+ * Managed pci_set_mwi().
+ *
+ * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
+ */
+int pcim_set_mwi(struct pci_dev *dev)
+{
+ struct pci_devres *dr;
+ int ret;
+
+ ret = pci_set_mwi(dev);
+ if (ret)
+ return ret;
+
+ dr = find_pci_dr(dev);
+ if (dr)
+ dr->mwi = 1;
+
+ return 0;
+}
+EXPORT_SYMBOL(pcim_set_mwi);
+
/**
* pci_try_set_mwi - enables memory-write-invalidate PCI transaction
* @dev: the PCI device for which MWI is enabled
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 978aad784..0a7ac863a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1064,6 +1064,7 @@ int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
int pci_set_cacheline_size(struct pci_dev *dev);
#define HAVE_PCI_SET_MWI
int __must_check pci_set_mwi(struct pci_dev *dev);
+int __must_check pcim_set_mwi(struct pci_dev *dev);
int pci_try_set_mwi(struct pci_dev *dev);
void pci_clear_mwi(struct pci_dev *dev);
void pci_intx(struct pci_dev *dev, int enable);
--
2.15.1
^ permalink raw reply related
* [PATCH 0/3] r8169: extend PCI core and switch to device-managed functions in probe
From: Heiner Kallweit @ 2017-12-09 23:30 UTC (permalink / raw)
To: Realtek linux nic maintainers, Bjorn Helgaas
Cc: netdev@vger.kernel.org, linux-pci@vger.kernel.org
Probe error path and remove callback can be significantly simplified
by using device-managed functions. To be able to do this in the r8169
driver we need a device-managed version of pci_set_mwi first.
Heiner Kallweit (3):
PCI: introduce device-managed version of pci_set_mwi
r8169: switch to device-managed functions in probe
r8169: remove netif_napi_del in probe error path
drivers/net/ethernet/realtek/r8169.c | 87 +++++++++---------------------------
drivers/pci/pci.c | 29 ++++++++++++
include/linux/pci.h | 1 +
3 files changed, 50 insertions(+), 67 deletions(-)
--
2.15.1
^ permalink raw reply
* Re: [PATCH net-next 2/2 v6] net: ethernet: Add a driver for Gemini gigabit ethernet
From: Linus Walleij @ 2017-12-09 23:27 UTC (permalink / raw)
To: Michał Mirosław
Cc: netdev, David S . Miller, Janos Laube, Paulius Zaleckas,
Linux ARM, Hans Ulli Kroll, Florian Fainelli, Tobias Waldvogel
In-Reply-To: <20171205190331.GA29007@qmqm.qmqm.pl>
On Tue, Dec 5, 2017 at 8:03 PM, Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> I'm happy to see that my work didn't go to /dev/null after all.
> I haven't finished it at the time because the box I had broke down
> beyond repair.
Ooops that explains why the submission was just haning in mid-air.
Sorry man :(
> I skimmed through the patch - please find my comments below.
Thanks a lot! :)
I fixed most of them for v8, just some comments:
> [...]
>> +static struct sk_buff *gmac_skb_if_good_frame(struct gemini_ethernet_port *port,
>> + GMAC_RXDESC_0_T word0, unsigned frame_len)
>> +{
>> + struct sk_buff *skb = NULL;
>> + unsigned rx_status = word0.bits.status;
>> + unsigned rx_csum = word0.bits.chksum_status;
>
>> + port->rx_stats[rx_status]++;
>> + port->rx_csum_stats[rx_csum]++;
>> +
>> + if (word0.bits.derr || word0.bits.perr ||
>> + rx_status || frame_len < ETH_ZLEN ||
>> + rx_csum >= RX_CHKSUM_IP_ERR_UNKNOWN) {
>> + port->stats.rx_errors++;
>> +
>> + if (frame_len < ETH_ZLEN || RX_ERROR_LENGTH(rx_status))
>> + port->stats.rx_length_errors++;
>> + if (RX_ERROR_OVER(rx_status))
>> + port->stats.rx_over_errors++;
>> + if (RX_ERROR_CRC(rx_status))
>> + port->stats.rx_crc_errors++;
>> + if (RX_ERROR_FRAME(rx_status))
>> + port->stats.rx_frame_errors++;
>> +
>> + return NULL;
>
> Could support RXALL feature here.
Probably! I might experiment with it in a separate (follow up) patch
since I have to figure out how much the hardware supports ignoring
errors and exploit that properly.
>> +static unsigned int gmac_rx(struct net_device *netdev, unsigned budget)
>> +{
> [...]
>> + if (unlikely(!mapping)) {
>> + netdev_err(netdev,
>> + "rxq[%u]: HW BUG: zero DMA desc\n", r);
>> + goto err_drop;
>> + }
>
> I wonder if this was a bug in the driver or in HW. Does it trigger on
> your boxes?
No I haven't seen it. I think it may be a HW bug on elder chips
(before SL3512 and SL3516) that is just not manifesting on newer
hardware.
Better keep the code though.
>> +static void gmac_set_rx_mode(struct net_device *netdev)
>> +{
>> + struct gemini_ethernet_port *port = netdev_priv(netdev);
>> + struct netdev_hw_addr *ha;
>> + __u32 mc_filter[2];
>> + unsigned bit_nr;
>> + GMAC_RX_FLTR_T filter = { .bits = {
>> + .broadcast = 1,
>> + .multicast = 1,
>> + .unicast = 1,
>> + } };
>> +
>> + mc_filter[1] = mc_filter[0] = 0;
>
> Looks like this should be = ~0u (IFF_ALLMULTI case).
Yeah it's some error compared to the (horrible) vendor code
here. The vendor tree explicitly checks for both promiscuous
and multicast and sets the masks to ~0 in both cases
explicitly, else leave it as default 0 with the funky
algorithm to set up the mask bit by bit.
I rewrote this piece of code to do the same.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH RFC 3/4] net: phy: add unlocked accessors
From: Russell King - ARM Linux @ 2017-12-09 23:11 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Andrew Lunn, netdev
In-Reply-To: <58260f24-a49c-fb1d-b774-fe457141a2d9@gmail.com>
On Sat, Dec 09, 2017 at 10:22:14AM -0800, Florian Fainelli wrote:
>
>
> On 12/08/2017 07:48 AM, Russell King wrote:
> > Add unlocked versions of the bus accessors, which allows access to the
> > bus with all the tracing. These accessors validate that the bus mutex
> > is held, which is a basic requirement for all mii bus accesses.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>
> > ---
> > include/linux/phy.h | 26 ++++++++++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/include/linux/phy.h b/include/linux/phy.h
> > index 71d777fe6c3d..964803bd7324 100644
> > --- a/include/linux/phy.h
> > +++ b/include/linux/phy.h
> > @@ -716,6 +716,18 @@ static inline int phy_read(struct phy_device *phydev, u32 regnum)
> > }
> >
> > /**
> > + * __phy_read - convenience function for reading a given PHY register
> > + * @phydev: the phy_device struct
> > + * @regnum: register number to read
> > + *
> > + * The caller must have taken the MDIO bus lock.
> > + */
> > +static inline int __phy_read(struct phy_device *phydev, u32 regnum)
>
> Do you know if we could have sparse validate that the caller of these
> functions holds the mutex? I remember reading somewhere that sparse does
> not do that yet, but can't get my hands on it.
Hi Florian,
I tried adding __acquires() and __releases() annotations, but sparse
complained about unbalanced lock counts when I did, because sparse has
no knowledge about mutexes being taken or released.
Unfortunately, it seems that these annotations are undocumented in the
kernel, but from what I can see, it turns out that they only apply to
spinlocks.
--
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 v3 2/5] net: Disable GRO_HW when generic XDP is installed on a device.
From: Alexander Duyck @ 2017-12-09 22:37 UTC (permalink / raw)
To: Michael Chan
Cc: David Miller, Netdev, Andrew Gospodarek, Ariel Elior,
everest-linux-l2
In-Reply-To: <CACKFLinJpXPScz07LdOrd765=eM7Gfg_57TL-ALDefMDkjdxJA@mail.gmail.com>
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:
>> On Fri, Dec 8, 2017 at 10:27 PM, Michael Chan <michael.chan@broadcom.com> wrote:
>>> Hardware should not aggregate any packets when generic XDP is installed.
>>>
>>> Cc: Ariel Elior <Ariel.Elior@cavium.com>
>>> Cc: everest-linux-l2@cavium.com
>>> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
>>> ---
>>> net/core/dev.c | 24 ++++++++++++++++++++++++
>>> 1 file changed, 24 insertions(+)
>>>
>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>> index 6ebd0e7..ec08ace 100644
>>> --- a/net/core/dev.c
>>> +++ b/net/core/dev.c
>>> @@ -1542,6 +1542,29 @@ void dev_disable_lro(struct net_device *dev)
>>> }
>>> EXPORT_SYMBOL(dev_disable_lro);
>>>
>>> +/**
>>> + * dev_disable_gro_hw - disable HW Generic Receive Offload on a device
>>> + * @dev: device
>>> + *
>>> + * Disable HW Generic Receive Offload (GRO_HW) on a net device. Must be
>>> + * called under RTNL. This is needed if Generic XDP is installed on
>>> + * the device.
>>> + */
>>> +static void dev_disable_gro_hw(struct net_device *dev)
>>> +{
>>> + struct net_device *lower_dev;
>>> + struct list_head *iter;
>>> +
>>> + dev->wanted_features &= ~NETIF_F_GRO_HW;
>>> + netdev_update_features(dev);
>>> +
>>> + if (unlikely(dev->features & NETIF_F_GRO_HW))
>>> + netdev_WARN(dev, "failed to disable GRO_HW!\n");
>>> +
>>> + netdev_for_each_lower_dev(dev, lower_dev, iter)
>>> + dev_disable_gro_hw(lower_dev);
>>
>> 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.
^ permalink raw reply
* Re: Incorrect source IP address on IGMP membership report
From: Andrew Lunn @ 2017-12-09 22:20 UTC (permalink / raw)
To: Kevin Cernekee; +Cc: netdev
In-Reply-To: <CAJzqFtZW=kmebXZAvkpkKv9jcvEeb5YrSjQ4odCXUnkHwnO0Rw@mail.gmail.com>
> Hmm, RFC3376 says:
>
> 4.2.13. IP Source Addresses for Reports
>
> An IGMP report is sent with a valid IP source address for the
> destination subnet. The 0.0.0.0 source address may be used by a
> system that has not yet acquired an IP address. Note that the
> 0.0.0.0 source address may simultaneously be used by multiple systems
> on a LAN. Routers MUST accept a report with a source address of
> 0.0.0.0.
>
> Would it make sense to add a special case that says "zero out
> pip->saddr if the interface doesn't have any IPv4 addresses"? e.g.
Yes, this looks O.K. for IGMPv3.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 1/5] net: Introduce NETIF_F_GRO_HW.
From: Alexander Duyck @ 2017-12-09 22:04 UTC (permalink / raw)
To: Michael Chan
Cc: David Miller, Netdev, Andrew Gospodarek, Ariel Elior,
everest-linux-l2
In-Reply-To: <CACKFLikDKvkq449A6=PFSsJrUpgfeogutawsW=nDApVfib92CQ@mail.gmail.com>
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:
>> On Fri, Dec 8, 2017 at 10:27 PM, Michael Chan <michael.chan@broadcom.com> wrote:
>>> Introduce NETIF_F_GRO_HW feature flag for NICs that support hardware
>>> GRO. With this flag, we can now independently turn on or off hardware
>>> GRO when GRO is on. Previously, drivers were using NETIF_F_GRO to
>>> control hardware GRO and so it cannot be independently turned on or
>>> off without affecting GRO.
>>>
>>> Hardware GRO (just like GRO) guarantees that packets can be re-segmented
>>> by TSO/GSO to reconstruct the original packet stream. It is a subset of
>>> NETIF_F_GRO and depends on it, as well as NETIF_F_RXCSUM.
>>
>> 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.
>> In addition as I said in my earlier comments I think we should
>> probably look at using this new feature bit to indicate that we allow
>> GRO to occur at or below this device as opposed to just above it as
>> currently occurs with conventional GRO.
>>
>>> Since NETIF_F_GRO is not propagated between upper and lower devices,
>>> NETIF_F_GRO_HW should follow suit since it is a subset of GRO. In other
>>> words, a lower device can independent have GRO/GRO_HW enabled or disabled
>>> and no feature propagation is required. This will preserve the current
>>> GRO behavior.
>>
>> 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.
>>> @@ -7424,6 +7424,18 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
>>> features &= ~dev->gso_partial_features;
>>> }
>>>
>>> + if (features & NETIF_F_GRO_HW) {
>>> + /* Hardware GRO depends on GRO and RXCSUM. */
>>> + if (!(features & NETIF_F_GRO)) {
>>> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no GRO feature.\n");
>>> + features &= ~NETIF_F_GRO_HW;
>>> + }
>>
>> 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, and you take a
performance penalty for trying to offload in software what you would
have already been handled in hardware.
Also, when we start propagating this up to indicate it is active we
don't want to have the GRO dependency since it would just make things
more expensive since we only need to do GRO in software once.
>> It
>> also makes it so that it is much easier to compare the performance for
>> GRO_HW against just a pure software GRO since you could then enable
>> them independently. Software GRO can come at a cost, and leaving it
>> enabled when you want to do it all in hardware is just adding a
>> penalty of sorts since I know for many of my routing tests I normally
>> disable GRO as it has a significant per-packet cost for small packet
>> workloads.
>>
>>> + if (!(features & NETIF_F_RXCSUM)) {
>>> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no RXCSUM feature.\n");
>>> + features &= ~NETIF_F_GRO_HW;
>>> + }
>>
>> So I was thinking about this. For LRO it makes sense to disable it in
>> the case of RXCSUM being disabled since most implementations leave the
>> Rx checksum mangled. However for GRO I am not sure it makes complete
>> sense. For software GRO we perform checksum validation in either
>> tcp4_gro_receive or tcp6_gro_receive. Why should the hardware
>> implementation behave differently? When a GRO frame is assembled the
>> checksum is converted to CHECKSUM_PARTIAL anyway even if Rx checksum
>> validation is disabled for the driver.
>
> 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.
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.
^ 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-09 21:40 UTC (permalink / raw)
To: Alexander Duyck
Cc: David Miller, Netdev, Andrew Gospodarek, Ariel Elior,
everest-linux-l2
In-Reply-To: <CAKgT0UcbjF+c+eoNTDrp-JQvC5k3jZpNN3H1JCge00MzYG=rhA@mail.gmail.com>
On Sat, Dec 9, 2017 at 10:56 AM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Fri, Dec 8, 2017 at 10:27 PM, Michael Chan <michael.chan@broadcom.com> wrote:
>> Hardware should not aggregate any packets when generic XDP is installed.
>>
>> Cc: Ariel Elior <Ariel.Elior@cavium.com>
>> Cc: everest-linux-l2@cavium.com
>> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
>> ---
>> net/core/dev.c | 24 ++++++++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 6ebd0e7..ec08ace 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1542,6 +1542,29 @@ void dev_disable_lro(struct net_device *dev)
>> }
>> EXPORT_SYMBOL(dev_disable_lro);
>>
>> +/**
>> + * dev_disable_gro_hw - disable HW Generic Receive Offload on a device
>> + * @dev: device
>> + *
>> + * Disable HW Generic Receive Offload (GRO_HW) on a net device. Must be
>> + * called under RTNL. This is needed if Generic XDP is installed on
>> + * the device.
>> + */
>> +static void dev_disable_gro_hw(struct net_device *dev)
>> +{
>> + struct net_device *lower_dev;
>> + struct list_head *iter;
>> +
>> + dev->wanted_features &= ~NETIF_F_GRO_HW;
>> + netdev_update_features(dev);
>> +
>> + if (unlikely(dev->features & NETIF_F_GRO_HW))
>> + netdev_WARN(dev, "failed to disable GRO_HW!\n");
>> +
>> + netdev_for_each_lower_dev(dev, lower_dev, iter)
>> + dev_disable_gro_hw(lower_dev);
>
> 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.
> 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.
^ permalink raw reply
* Re: NFS corruption, fixed by echo 1 > /proc/sys/vm/drop_caches -- next debugging steps?
From: Eric Dumazet @ 2017-12-09 21:37 UTC (permalink / raw)
To: Matt Turner
Cc: Eric Dumazet, linux-mips@linux-mips.org, linux-nfs, Paolo Abeni,
Hannes Frederic Sowa, Peter Zijlstra (Intel), Thomas Gleixner,
Ingo Molnar, Manuel Lauss, LKML, netdev
In-Reply-To: <CAEdQ38H+jUF3OXpe13Vfm=QZE3iHa=B7PpXkpbek1PnY2E1u5w@mail.gmail.com>
On Sat, 2017-12-09 at 13:03 -0800, Matt Turner wrote:
> On Fri, Dec 8, 2017 at 1:16 PM, Eric Dumazet <eric.dumazet@gmail.com>
> wrote:
> > On Fri, 2017-12-08 at 12:26 -0800, Matt Turner wrote:
> > >
> > > Thanks for the quick reply!
> > >
> > > I tried the patch on top of master, but unfortunately the
> > > corruption
> > > still occurs.
> >
> > You might try replacing in sbdma_add_rcvbuffer()
> >
> > sb_new = netdev_alloc_skb(dev, size);
> >
> > by
> >
> > sb_new = alloc_skb(size, GFP_ATOMIC);
> >
> > Maybe the device does not like having a frame spanning 2 pages.
>
> No such luck. I also gave changing the page size from 16K to 4K a
> shot
> without success.
If your hist is SMP, could you try running it with one CPU only ?
Sorry, I have no more ideas :/
^ permalink raw reply
* Re: [PATCH net-next v3 1/5] net: Introduce NETIF_F_GRO_HW.
From: Michael Chan @ 2017-12-09 21:31 UTC (permalink / raw)
To: Alexander Duyck
Cc: David Miller, Netdev, Andrew Gospodarek, Ariel Elior,
everest-linux-l2
In-Reply-To: <CAKgT0Ue0hLC8PkJm2dpxvbS9hNsSEXxMFbyt0cu3LoZNYpi_ig@mail.gmail.com>
On Sat, Dec 9, 2017 at 10:50 AM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Fri, Dec 8, 2017 at 10:27 PM, Michael Chan <michael.chan@broadcom.com> wrote:
>> Introduce NETIF_F_GRO_HW feature flag for NICs that support hardware
>> GRO. With this flag, we can now independently turn on or off hardware
>> GRO when GRO is on. Previously, drivers were using NETIF_F_GRO to
>> control hardware GRO and so it cannot be independently turned on or
>> off without affecting GRO.
>>
>> Hardware GRO (just like GRO) guarantees that packets can be re-segmented
>> by TSO/GSO to reconstruct the original packet stream. It is a subset of
>> NETIF_F_GRO and depends on it, as well as NETIF_F_RXCSUM.
>
> 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).
> In addition as I said in my earlier comments I think we should
> probably look at using this new feature bit to indicate that we allow
> GRO to occur at or below this device as opposed to just above it as
> currently occurs with conventional GRO.
>
>> Since NETIF_F_GRO is not propagated between upper and lower devices,
>> NETIF_F_GRO_HW should follow suit since it is a subset of GRO. In other
>> words, a lower device can independent have GRO/GRO_HW enabled or disabled
>> and no feature propagation is required. This will preserve the current
>> GRO behavior.
>
> 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.
>> @@ -7424,6 +7424,18 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
>> features &= ~dev->gso_partial_features;
>> }
>>
>> + if (features & NETIF_F_GRO_HW) {
>> + /* Hardware GRO depends on GRO and RXCSUM. */
>> + if (!(features & NETIF_F_GRO)) {
>> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no GRO feature.\n");
>> + features &= ~NETIF_F_GRO_HW;
>> + }
>
> 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.
> It
> also makes it so that it is much easier to compare the performance for
> GRO_HW against just a pure software GRO since you could then enable
> them independently. Software GRO can come at a cost, and leaving it
> enabled when you want to do it all in hardware is just adding a
> penalty of sorts since I know for many of my routing tests I normally
> disable GRO as it has a significant per-packet cost for small packet
> workloads.
>
>> + if (!(features & NETIF_F_RXCSUM)) {
>> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no RXCSUM feature.\n");
>> + features &= ~NETIF_F_GRO_HW;
>> + }
>
> So I was thinking about this. For LRO it makes sense to disable it in
> the case of RXCSUM being disabled since most implementations leave the
> Rx checksum mangled. However for GRO I am not sure it makes complete
> sense. For software GRO we perform checksum validation in either
> tcp4_gro_receive or tcp6_gro_receive. Why should the hardware
> implementation behave differently? When a GRO frame is assembled the
> checksum is converted to CHECKSUM_PARTIAL anyway even if Rx checksum
> validation is disabled for the driver.
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.
^ permalink raw reply
* Re: [PATCH net-next v3 0/2] bpf/tracing: allow user space to query prog array on the same tp
From: Alexei Starovoitov @ 2017-12-09 21:21 UTC (permalink / raw)
To: Yonghong Song; +Cc: peterz, rostedt, ast, daniel, kafai, netdev, kernel-team
In-Reply-To: <20171206220746.4038383-1-yhs@fb.com>
On Wed, Dec 06, 2017 at 02:07:44PM -0800, Yonghong Song wrote:
> Commit e87c6bc3852b ("bpf: permit multiple bpf attachments
> for a single perf event") added support to attach multiple
> bpf programs to a single perf event. Given a perf event
> (kprobe, uprobe, or kernel tracepoint), the perf ioctl interface
> is used to query bpf programs attached to the same trace event.
>
> There already exists a BPF_PROG_QUERY command for introspection
> currently used by cgroup+bpf. We did have an implementation for
> querying tracepoint+bpf through the same interface. However, it
> looks cleaner to use ioctl() style of api here, since attaching
> bpf prog to tracepoint/kuprobe is also done via ioctl.
>
> Patch #1 had the core implementation and patch #2 added
> a test case in tools bpf selftests suite.
>
> Changelogs:
> v2 -> v3:
> - Change uapi structure perf_event_query_bpf to be more
> clearer based on Peter's suggestion, and adjust
> other codes accordingly.
Peter,
could you please ack the patch 1 ?
We'd like to route the set via bpf-next -> net-next trees.
^ permalink raw reply
* Re: NFS corruption, fixed by echo 1 > /proc/sys/vm/drop_caches -- next debugging steps?
From: Matt Turner @ 2017-12-09 21:03 UTC (permalink / raw)
To: Eric Dumazet
Cc: Eric Dumazet, linux-mips-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, Paolo Abeni,
Hannes Frederic Sowa, Peter Zijlstra (Intel), Thomas Gleixner,
Ingo Molnar, Manuel Lauss, LKML, netdev
In-Reply-To: <1512767781.25033.30.camel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Fri, Dec 8, 2017 at 1:16 PM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, 2017-12-08 at 12:26 -0800, Matt Turner wrote:
>>
>> Thanks for the quick reply!
>>
>> I tried the patch on top of master, but unfortunately the corruption
>> still occurs.
>
> You might try replacing in sbdma_add_rcvbuffer()
>
> sb_new = netdev_alloc_skb(dev, size);
>
> by
>
> sb_new = alloc_skb(size, GFP_ATOMIC);
>
> Maybe the device does not like having a frame spanning 2 pages.
No such luck. I also gave changing the page size from 16K to 4K a shot
without success.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net-next v2 6/6] net: qualcomm: rmnet: Allow to configure flags for existing devices
From: Subash Abhinov Kasiviswanathan @ 2017-12-09 20:58 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1512853110-4893-1-git-send-email-subashab@codeaurora.org>
Add an option to configure the mux id, aggregation and commad feature
for existing rmnet devices. Implement the changelink netlink
operation for this.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 40 ++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 2f5f661..3ec4092 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -319,6 +319,45 @@ static int rmnet_rtnl_validate(struct nlattr *tb[], struct nlattr *data[],
return 0;
}
+static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[],
+ struct nlattr *data[],
+ struct netlink_ext_ack *extack)
+{
+ struct rmnet_priv *priv = netdev_priv(dev);
+ struct net_device *real_dev;
+ struct rmnet_endpoint *ep;
+ struct rmnet_port *port;
+ u16 mux_id;
+
+ real_dev = __dev_get_by_index(dev_net(dev),
+ nla_get_u32(tb[IFLA_LINK]));
+
+ if (!real_dev || !dev || !rmnet_is_real_dev_registered(real_dev))
+ return -ENODEV;
+
+ port = rmnet_get_port_rtnl(real_dev);
+
+ if (data[IFLA_VLAN_ID]) {
+ mux_id = nla_get_u16(data[IFLA_VLAN_ID]);
+ ep = rmnet_get_endpoint(port, priv->mux_id);
+
+ hlist_del_init_rcu(&ep->hlnode);
+ hlist_add_head_rcu(&ep->hlnode, &port->muxed_ep[mux_id]);
+
+ ep->mux_id = mux_id;
+ priv->mux_id = mux_id;
+ }
+
+ if (data[IFLA_VLAN_FLAGS]) {
+ struct ifla_vlan_flags *flags;
+
+ flags = nla_data(data[IFLA_VLAN_FLAGS]);
+ port->ingress_data_format = flags->flags & flags->mask;
+ }
+
+ return 0;
+}
+
static size_t rmnet_get_size(const struct net_device *dev)
{
return nla_total_size(2) /* IFLA_VLAN_ID */ +
@@ -334,6 +373,7 @@ struct rtnl_link_ops rmnet_link_ops __read_mostly = {
.newlink = rmnet_newlink,
.dellink = rmnet_dellink,
.get_size = rmnet_get_size,
+ .changelink = rmnet_changelink,
};
/* Needs either rcu_read_lock() or rtnl lock */
--
1.9.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