* Re: [PATCH net-next V3 2/2] rtnl: Add support for netdev event attribute to link messages
From: Roopa Prabhu @ 2017-04-24 15:14 UTC (permalink / raw)
To: David Ahern
Cc: Vladislav Yasevich, netdev@vger.kernel.org, Vladislav Yasevich,
Jiri Pirko
In-Reply-To: <877efb54-2aef-4d1e-c0b4-2ce6aa6562df@cumulusnetworks.com>
On Sun, Apr 23, 2017 at 6:07 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
>
> On 4/21/17 11:31 AM, Vladislav Yasevich wrote:
> > @@ -1276,9 +1277,40 @@ static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
> > return err;
> > }
> >
> > +static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
> > +{
> > + u32 rtnl_event;
> > +
> > + switch (event) {
> > + case NETDEV_REBOOT:
> > + rtnl_event = IFLA_EVENT_REBOOT;
> > + break;
> > + case NETDEV_FEAT_CHANGE:
> > + rtnl_event = IFLA_EVENT_FEAT_CHANGE;
> > + break;
> > + case NETDEV_BONDING_FAILOVER:
> > + rtnl_event = IFLA_EVENT_BONDING_FAILOVER;
> > + break;
> > + case NETDEV_NOTIFY_PEERS:
> > + rtnl_event = IFLA_EVENT_NOTIFY_PEERS;
> > + break;
> > + case NETDEV_RESEND_IGMP:
> > + rtnl_event = IFLA_EVENT_RESEND_IGMP;
> > + break;
> > + case NETDEV_CHANGEINFODATA:
> > + rtnl_event = IFLA_EVENT_CHANGE_INFO_DATA;
> > + break;
> > + default:
> > + return 0;
> > + }
> > +
> > + return nla_put_u32(skb, IFLA_EVENT, rtnl_event);
> > +}
> > +
>
> I still have doubts about encoding kernel events into a uapi.
agree. I don't see why user-space will need NETDEV_CHANGEINFODATA and
others david listed.
My other concerns are, once we have this exposed to user-space and
user-space starts relying on it, it will need accurate information and
will expect to have this event information all the time.
IIUC, we cannot cover multiple events in a single notification and not
all link notifications will contain an IFLA_EVENT attribute. In other
words, we will be telling user-space to not expect that the kernel
will send IFLA_EVENT every time.
>
> For example, NETDEV_CHANGEINFODATA is only for bonds though nothing
> about the name suggests it is a bonding notification. This one was added
> specifically to notify userspace (d4261e5650004), yet seems to happen
> only during a changelink and that already generates a RTM_NEWLINK
> message via do_setlink. Since the rtnetlink_event message does not
> contain anything "NETDEV_CHANGEINFODATA" related what purpose does it
> really serve besides duplicating netlink messages to userspace.
>
> The REBOOT, IGMP, FEAT_CHANGE and BONDING_FAILOVER seem to be unique
> messages (code analysis only) which I get for notifying userspace.
>
> NETDEV_NOTIFY_PEERS is not so clear in how often it duplicates other
> messages.
^ permalink raw reply
* Re: [RFC PATCH 3/7] net: add option to get information about timestamped packets
From: Willem de Bruijn @ 2017-04-24 15:18 UTC (permalink / raw)
To: Miroslav Lichvar
Cc: Network Development, Richard Cochran, Willem de Bruijn,
Soheil Hassas Yeganeh, Keller, Jacob E, Denny Page, Jiri Benc
In-Reply-To: <20170424090043.GF8847@localhost>
On Mon, Apr 24, 2017 at 5:00 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
> On Thu, Apr 13, 2017 at 12:16:09PM -0400, Willem de Bruijn wrote:
>> On Thu, Apr 13, 2017 at 11:18 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
>> > On Thu, Apr 13, 2017 at 10:37:07AM -0400, Willem de Bruijn wrote:
>> >> Why is this L2 length needed?
>> >
>> > It's needed for incoming packets to allow converting of preamble
>> > timestamps to trailer timestamps.
>>
>> Receiving the mac length of a packet sounds like a feature independent
>> from timestamping.
>
> I agree, but so far nobody suggested another use for this information.
> Do you have any suggestions?
>
> The idea was that if it is useful only with HW timestamping, it would
> be better to save it only with the timestamp, so there is no
> performance impact in the more common case when HW timestamping is
> disabled. Am I overly cautious here?
The additional cost of a cmsg is zero for sockets that have no cmsg
enabled, due to
if (inet->cmsg_flags)
ip_cmsg_recv_offset(msg, sk, skb, sizeof(struct udphdr), off);
But you might be right that there are no uses outside the specific
timestamp requirement you have, so if you prefer to use a timestamp
option, I won't object further.
>> Either an ioctl similar to SIOCGIFMTU or, if it may
>> vary due to existince of vlan headers, a new independent cmsg at the
>> SOL_SOCKET layer.
The latter would require adding the SOL_SOCKET level cmsg processing
infra. It is simpler to just add it at the INET/INET6 levels.
> It's not just the VLAN headers. The length of the IP header may vary
> with IP options, so the offset of the UDP data in the packet cannot be
> assumed to be constant.
As well as tunnels.
> Now I'm wondering if it's actually necessary to save the original
> value of skb->mac_len + skb->len.
Computing it on recv if needed is definitely preferable to computing
on enqueue and storing in an intermediate variable.
> Would "skb->data - skb->head -
> skb->mac_header + skb->len" always work as the L2 length for received
> packets at the time when the cmsg is prepared?
(skb->data - skb->head) - skb->mac_header computes the length
of data before the mac, such as reserve? Do you mean skb->data -
skb->mac_header (or - skb_mac_offset(skb))?
> As for the original ifindex, it seems to me it does need to be saved
> to a new field since __netif_receive_skb_core() intentionally
> overwrites skb->skb_iif. What would be the best place for it, sk_buff
> or skb_shared_info?
Finding storage space on the receive path will not be easy.
One shortcut to avoid storing this information explicitly is to look up
the device from skb->napi_id.
> And would it really be acceptable to save it for all packets in
> __netif_receive_skb_core(), even when HW timestamping is disabled?
> Seeing how the code and the data structures were optimized over time,
> I have a feeling it would not be accepted.
Incurring this cost on all packets for such a rare edge case does sound
like a non-starter.
It can be called only if the netstamp_needed static key is enabled (false),
in __net_timestamp, though.
^ permalink raw reply
* Re: [PATCH net] bridge: shutdown bridge device before removing it
From: Xin Long @ 2017-04-24 15:21 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: network dev, bridge@lists.linux-foundation.org, David S. Miller
In-Reply-To: <940af2e3-8742-46b0-0550-0bba5e0a3f71@cumulusnetworks.com>
On Mon, Apr 24, 2017 at 10:53 PM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> On 24/04/17 17:41, Xin Long wrote:
>> On Mon, Apr 24, 2017 at 8:07 PM, Nikolay Aleksandrov
>> <nikolay@cumulusnetworks.com> wrote:
>>> On 24/04/17 14:01, Nikolay Aleksandrov wrote:
>>>> On 24/04/17 10:25, Xin Long wrote:
>>>>> During removing a bridge device, if the bridge is still up, a new mdb entry
>>>>> still can be added in br_multicast_add_group() after all mdb entries are
>>>>> removed in br_multicast_dev_del(). Like the path:
>>>>>
>>>>> mld_ifc_timer_expire ->
>>>>> mld_sendpack -> ...
>>>>> br_multicast_rcv ->
>>>>> br_multicast_add_group
>>>>>
>>>>> The new mp's timer will be set up. If the timer expires after the bridge
>>>>> is freed, it may cause use-after-free panic in br_multicast_group_expired.
>>>>> This can happen when ip link remove a bridge or destroy a netns with a
>>>>> bridge device inside.
>>>>>
>>>>> As we can see in br_del_bridge, brctl is also supposed to remove a bridge
>>>>> device after it's shutdown.
>>>>>
>>>>> This patch is to call dev_close at the beginning of br_dev_delete so that
>>>>> netif_running check in br_multicast_add_group can avoid this issue. But
>>>>> to keep consistent with before, it will not remove the IFF_UP check in
>>>>> br_del_bridge for brctl.
>>>>>
>>>>> Reported-by: Jianwen Ji <jiji@redhat.com>
>>>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>>>>> ---
>>>>> net/bridge/br_if.c | 2 ++
>>>>> 1 file changed, 2 insertions(+)
>>>>>
>>>>
>>>> +CC bridge maintainers
>>>>
>>>> I can see how this could happen, could you also provide the traceback ?
>>>>
>>>> The patch looks good to me, actually I think it fixes another issue with
>>>> mcast stats where the percpu pointer can be accessed after it's freed if
>>>> an mcast packet can get sent via br->dev after the br_multicast_dev_del() call.
>>>> This is definitely stable material, if I'm not mistaken the issue is there since
>>>> the introduction of br_dev_delete:
>>>> commit e10177abf842
>>>> Author: Satish Ashok <sashok@cumulusnetworks.com>
>>>> Date: Wed Jul 15 07:16:51 2015 -0700
>>>>
>>>> bridge: multicast: fix handling of temp and perm entries
>>>>
>>>>
>>>>
>>>> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>>>>
>>>
>>> Actually I have a better idea for a fix because dev_close() for a single device is rather heavy.
>>> Why don't you move the mdb flush logic in the bridge's ndo_uninit() callback ?
>>> That should have the same effect and be much faster.
>> Yes. But it seems that all cleanups for bridge should be done after
>> it's shutdown since beginning according to brctl. I'm not sure if there
>> are still other problems caused by this. maybe safer to use dev_close.
>> I need to check more to confirm this.
>>
>
> ndo_uninit() is after the device has been stopped, so it is the same as
> your fix as I said.
got that your suggestion can fix this issue. what I'm afraid of is there
are still other problems like this issue, like "the percpu pointer" one
you just mentioned above, though it's already fixed by ndo_uninit.
dev_close would just avoid ALL this kind of issues if there still are. :)
But if you can be sure no more issue like this one, I'm all for that,
will improve this patch with your suggestion.
>
>> I also have another question about mp->timer removing.
>> As we can see, now it removes this timer with del_timer, instead of
>> del_timer_sync. What if the timer is running when del_timer ?
>> How can we be sure that br_multicast_group_expired will be done
>> before the bridge dev is freed. synchronize_net ?
>>
>
> Yeah, I've been thinking about that and the only race is that the timer
> might have fired and waiting for the lock while the mdb is being flushed
> thus the cancel_timer() won't affect it and then it will enter and see
> that !netif_running(br->dev), but unfortunately there's a bug because we
> cannot guarantee that br->dev still exists at that point.
> This is a different bug though.
exactly, the bad thing is it's pretty hard to reproduce even if this bz exists,
since the timer process can not be preemptable. synchronize_net probably
could avoid it (not sure).
>
>>>
>>> By the way I just noticed that there's also a memory leak - the mdb hash is reallocated
>>> and not freed due to the mdb rehash, here's also kmemleak's object:
>>>
>> yeps, ;-)
>>
>>> unreferenced object 0xffff8800540ba800 (size 2048):
>>> comm "softirq", pid 0, jiffies 4520588901 (age 5787.284s)
>>> hex dump (first 32 bytes):
>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>>> backtrace:
>>> [<ffffffff816e2287>] kmemleak_alloc+0x67/0xc0
>>> [<ffffffff81260bea>] __kmalloc+0x1ba/0x3e0
>>> [<ffffffffa05c60ee>] br_mdb_rehash+0x5e/0x340 [bridge]
>>> [<ffffffffa05c74af>] br_multicast_new_group+0x43f/0x6e0 [bridge]
>>> [<ffffffffa05c7aa3>] br_multicast_add_group+0x203/0x260 [bridge]
>>> [<ffffffffa05ca4b5>] br_multicast_rcv+0x945/0x11d0 [bridge]
>>> [<ffffffffa05b6b10>] br_dev_xmit+0x180/0x470 [bridge]
>>> [<ffffffff815c781b>] dev_hard_start_xmit+0xbb/0x3d0
>>> [<ffffffff815c8743>] __dev_queue_xmit+0xb13/0xc10
>>> [<ffffffff815c8850>] dev_queue_xmit+0x10/0x20
>>> [<ffffffffa02f8d7a>] ip6_finish_output2+0x5ca/0xac0 [ipv6]
>>> [<ffffffffa02fbfc6>] ip6_finish_output+0x126/0x2c0 [ipv6]
>>> [<ffffffffa02fc245>] ip6_output+0xe5/0x390 [ipv6]
>>> [<ffffffffa032b92c>] NF_HOOK.constprop.44+0x6c/0x240 [ipv6]
>>> [<ffffffffa032bd16>] mld_sendpack+0x216/0x3e0 [ipv6]
>>> [<ffffffffa032d5eb>] mld_ifc_timer_expire+0x18b/0x2b0 [ipv6]
>>>
>>>
>>>
>
^ permalink raw reply
* Re: macvlan: Fix device ref leak when purging bc_queue
From: Joe.Ghalam @ 2017-04-24 15:30 UTC (permalink / raw)
To: davem; +Cc: herbert, Clifford.Wichmann, netdev
In-Reply-To: <20170424.111028.2157290275229080747.davem@davemloft.net>
> I'm waiting for this discussion to settle down before I apply the patch.
Thanks David. I will get some answers soon, and hopefully the change is a good one.
^ permalink raw reply
* [iproute PATCH] man: ip-rule.8: Further clarify how to interpret priority value
From: Phil Sutter @ 2017-04-24 15:35 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Despite the past changes, users seemed to get confused by the seemingly
contradictory relation of priority value and actual rule priority.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
man/man8/ip-rule.8 | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/man/man8/ip-rule.8 b/man/man8/ip-rule.8
index 7de80f3e6db9f..a5c479811927f 100644
--- a/man/man8/ip-rule.8
+++ b/man/man8/ip-rule.8
@@ -95,7 +95,10 @@ Each policy routing rule consists of a
.B selector
and an
.B action predicate.
-The RPDB is scanned in order of decreasing priority. The selector
+The RPDB is scanned in order of decreasing priority (note that lower number
+means higher priority, see the description of
+.I PREFERENCE
+below). The selector
of each rule is applied to {source address, destination address, incoming
interface, tos, fwmark} and, if the selector matches the packet,
the action is performed. The action predicate may return with success.
@@ -225,7 +228,8 @@ value to match.
.BI priority " PREFERENCE"
the priority of this rule.
.I PREFERENCE
-is an unsigned integer value, higher number means lower priority. Each rule
+is an unsigned integer value, higher number means lower priority, and rules get
+processed in order of increasing number. Each rule
should have an explicitly set
.I unique
priority value.
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v3 07/29] x86: bpf_jit, use ENTRY+ENDPROC
From: Jiri Slaby @ 2017-04-24 15:41 UTC (permalink / raw)
To: David Miller
Cc: alexei.starovoitov, mingo, tglx, hpa, x86, jpoimboe, linux-kernel,
netdev, daniel, edumazet
In-Reply-To: <20170424.110844.1321374394090353753.davem@davemloft.net>
On 04/24/2017, 05:08 PM, David Miller wrote:
> If you align the entry points, then the code sequence as a whole is
> are no longer densely packed.
Sure.
> Or do I misunderstand how your macros work?
Perhaps. So the suggested macros for the code are:
#define BPF_FUNC_START_LOCAL(name) \
SYM_START(name, SYM_V_LOCAL, SYM_A_NONE)
#define BPF_FUNC_START(name) \
SYM_START(name, SYM_V_GLOBAL, SYM_A_NONE)
and they differ from the standard ones:
#define SYM_FUNC_START_LOCAL(name) \
SYM_START(name, SYM_V_LOCAL, SYM_A_ALIGN)
#define SYM_FUNC_START(name) \
SYM_START(name, SYM_V_GLOBAL, SYM_A_ALIGN)
The difference is SYM_A_NONE vs. SYM_A_ALIGN, which means:
#define SYM_A_ALIGN ALIGN
#define SYM_A_NONE /* nothing */
Does it look OK now?
thanks,
--
js
suse labs
^ permalink raw reply
* Re: [PATCH v4] {net,IB}/{rxe,usnic}: Utilize generic mac to eui32 function
From: Yuval Shaia @ 2017-04-24 15:46 UTC (permalink / raw)
To: Leon Romanovsky, dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: benve-FYB4Gu1CFyUAvxtiuMwx3w, dgoodell-FYB4Gu1CFyUAvxtiuMwx3w,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
monis-VPRAkNaXOzVWk0Htik3J/w, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170314175843.GY2079-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
On Tue, Mar 14, 2017 at 07:58:43PM +0200, Leon Romanovsky wrote:
> On Tue, Mar 14, 2017 at 04:01:57PM +0200, Yuval Shaia wrote:
> > This logic seems to be duplicated in (at least) three separate files.
> > Move it to one place so code can be re-use.
> >
> > Signed-off-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> > v0 -> v1:
> > * Add missing #include
> > * Rename to genaddrconf_ifid_eui48
> > v1 -> v2:
> > * Reset eui[0] to default if dev_id is used
> > v2 -> v3:
> > * Add helper function to avoid re-setting eui[0] to default if
> > dev_id is used
> > v3 -> v4:
> > * Remove RXE wrappers
> > * Remove addrconf_addr_eui48_xor and do the eui[0] ^= 2 in the
> > basic implementation
> > ---
> > drivers/infiniband/hw/usnic/usnic_common_util.h | 11 +++-------
> > drivers/infiniband/sw/rxe/rxe.c | 4 +++-
> > drivers/infiniband/sw/rxe/rxe_loc.h | 2 --
> > drivers/infiniband/sw/rxe/rxe_net.c | 28 -------------------------
> > drivers/infiniband/sw/rxe/rxe_verbs.c | 4 +++-
> > include/net/addrconf.h | 22 +++++++++++++++----
> > 6 files changed, 27 insertions(+), 44 deletions(-)
> >
>
> Thanks, Yuval.
> Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Hi Doug,
If no more comments on this one can you consider taking it?
Thanks,
Yuval
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 07/29] x86: bpf_jit, use ENTRY+ENDPROC
From: David Miller @ 2017-04-24 15:51 UTC (permalink / raw)
To: jslaby
Cc: alexei.starovoitov, mingo, tglx, hpa, x86, jpoimboe, linux-kernel,
netdev, daniel, edumazet
In-Reply-To: <614ca52b-8a43-244e-8a3a-c39145ecc3e8@suse.cz>
From: Jiri Slaby <jslaby@suse.cz>
Date: Mon, 24 Apr 2017 17:41:06 +0200
> On 04/24/2017, 05:08 PM, David Miller wrote:
>> If you align the entry points, then the code sequence as a whole is
>> are no longer densely packed.
>
> Sure.
>
>> Or do I misunderstand how your macros work?
>
> Perhaps. So the suggested macros for the code are:
> #define BPF_FUNC_START_LOCAL(name) \
> SYM_START(name, SYM_V_LOCAL, SYM_A_NONE)
> #define BPF_FUNC_START(name) \
> SYM_START(name, SYM_V_GLOBAL, SYM_A_NONE)
>
> and they differ from the standard ones:
> #define SYM_FUNC_START_LOCAL(name) \
> SYM_START(name, SYM_V_LOCAL, SYM_A_ALIGN)
> #define SYM_FUNC_START(name) \
> SYM_START(name, SYM_V_GLOBAL, SYM_A_ALIGN)
>
>
> The difference is SYM_A_NONE vs. SYM_A_ALIGN, which means:
> #define SYM_A_ALIGN ALIGN
> #define SYM_A_NONE /* nothing */
>
> Does it look OK now?
I said I'm not OK with the alignment, so personally I am not
with how these macros work and what they will do to the code
generated for BPF packet accesses.
But I'll defer to Alexei on this because I don't have the time
nor the energy to fight this.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 07/29] x86: bpf_jit, use ENTRY+ENDPROC
From: Jiri Slaby @ 2017-04-24 15:53 UTC (permalink / raw)
To: David Miller
Cc: alexei.starovoitov, mingo, tglx, hpa, x86, jpoimboe, linux-kernel,
netdev, daniel, edumazet
In-Reply-To: <20170424.115118.1652158849030310645.davem@davemloft.net>
On 04/24/2017, 05:51 PM, David Miller wrote:
> I said I'm not OK with the alignment
So in short, the suggested macros add no alignment.
--
js
suse labs
^ permalink raw reply
* Re: [net-next 0/7][pull request] 1GbE Intel Wired LAN Driver Updates 2017-04-20
From: David Miller @ 2017-04-24 15:54 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20170420233335.34900-1-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 20 Apr 2017 16:33:28 -0700
> This series contains updates to e1000, e1000e, igb/vf and ixgb.
Pulled, thanks Jeff.
^ permalink raw reply
* Re: I find one aspect of the ip link show command confusing and I'd like you to fix it, please
From: Stephen Hemminger @ 2017-04-24 15:54 UTC (permalink / raw)
To: Jeff Silverman; +Cc: netdev
In-Reply-To: <CAGu9dLLRVaiO-vHd_jzVuMM3O=sjLX_VOeu5n6-+eV6fEdhBug@mail.gmail.com>
On Sun, 23 Apr 2017 15:36:32 -0700
Jeff Silverman <jeffsilverm@gmail.com> wrote:
> People,
>
> When my NIC is up, but not connected, I see:
>
> root@jeff-desktop:~# ip link show enp3s0
> 2: enp3s0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc mq state
> DOWN mode DEFAULT group default qlen 1000
> link/ether 00:10:18:cc:9c:77 brd ff:ff:ff:ff:ff:ff
> root@jeff-desktop:~# ip link show enp3s0
>
> NO-CARRIER makes sense to me - if the wire is unplugged, then the NIC
> isn't seeing the humm at the beginning of each packet. That's clear.
> Note that even though my link isn't plugged in, ip still notes that
> it is up. That's great.
>
>
> But if I down my NIC, there is no indication that it is DOWN other
> than you can't see the UP flag. If somebody was new to linux, they
> would not see what's not there.
>
> 2: enp3s0: <BROADCAST,MULTICAST> mtu 1500 qdisc mq state DOWN mode
> DEFAULT group default qlen 1000
> link/ether 00:10:18:cc:9c:77 brd ff:ff:ff:ff:ff:ff
>
>
> What I would like you to do is modify the ip command so that if the
> NIC has been downed by something, then it explicitly says DOWN.
>
> What would be really nice would be if you enumerated all of the flags
> that an interface can have, and note if the flag is set or cleared.
> But that's more than what I want with this message.
>
>
> Many thanks,
>
>
> Jeff
>
If you have a suggestion send a patch. The utility has shown the same
output since the earliest versions. Therefore the default output format
can't change since people do things like write scripts to parse it.
A more verbose output is possible but would have to be enabled by
a flag.
^ permalink raw reply
* Re: [PATCH v3 07/29] x86: bpf_jit, use ENTRY+ENDPROC
From: Ingo Molnar @ 2017-04-24 15:55 UTC (permalink / raw)
To: Jiri Slaby
Cc: David Miller, alexei.starovoitov, mingo, tglx, hpa, x86, jpoimboe,
linux-kernel, netdev, daniel, edumazet
In-Reply-To: <614ca52b-8a43-244e-8a3a-c39145ecc3e8@suse.cz>
* Jiri Slaby <jslaby@suse.cz> wrote:
> On 04/24/2017, 05:08 PM, David Miller wrote:
> > If you align the entry points, then the code sequence as a whole is
> > are no longer densely packed.
>
> Sure.
>
> > Or do I misunderstand how your macros work?
>
> Perhaps. So the suggested macros for the code are:
> #define BPF_FUNC_START_LOCAL(name) \
> SYM_START(name, SYM_V_LOCAL, SYM_A_NONE)
> #define BPF_FUNC_START(name) \
> SYM_START(name, SYM_V_GLOBAL, SYM_A_NONE)
>
> and they differ from the standard ones:
> #define SYM_FUNC_START_LOCAL(name) \
> SYM_START(name, SYM_V_LOCAL, SYM_A_ALIGN)
> #define SYM_FUNC_START(name) \
> SYM_START(name, SYM_V_GLOBAL, SYM_A_ALIGN)
>
>
> The difference is SYM_A_NONE vs. SYM_A_ALIGN, which means:
> #define SYM_A_ALIGN ALIGN
> #define SYM_A_NONE /* nothing */
>
> Does it look OK now?
No, the patch changes alignment which is undesirable, it needs to preserve the
existing (non-)alignment of the symbols!
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH net] bridge: shutdown bridge device before removing it
From: Nikolay Aleksandrov @ 2017-04-24 15:55 UTC (permalink / raw)
To: Xin Long
Cc: network dev, bridge@lists.linux-foundation.org, David S. Miller,
Herbert Xu
In-Reply-To: <CADvbK_fJAqpcspMO7SPhRcbia9KLBA=Q5E2ySTsV0A6+EfDzFg@mail.gmail.com>
On 24/04/17 18:21, Xin Long wrote:
> On Mon, Apr 24, 2017 at 10:53 PM, Nikolay Aleksandrov
> <nikolay@cumulusnetworks.com> wrote:
>> On 24/04/17 17:41, Xin Long wrote:
>>> On Mon, Apr 24, 2017 at 8:07 PM, Nikolay Aleksandrov
>>> <nikolay@cumulusnetworks.com> wrote:
>>>> On 24/04/17 14:01, Nikolay Aleksandrov wrote:
>>>>> On 24/04/17 10:25, Xin Long wrote:
>>>>>> During removing a bridge device, if the bridge is still up, a new mdb entry
>>>>>> still can be added in br_multicast_add_group() after all mdb entries are
>>>>>> removed in br_multicast_dev_del(). Like the path:
>>>>>>
>>>>>> mld_ifc_timer_expire ->
>>>>>> mld_sendpack -> ...
>>>>>> br_multicast_rcv ->
>>>>>> br_multicast_add_group
>>>>>>
>>>>>> The new mp's timer will be set up. If the timer expires after the bridge
>>>>>> is freed, it may cause use-after-free panic in br_multicast_group_expired.
>>>>>> This can happen when ip link remove a bridge or destroy a netns with a
>>>>>> bridge device inside.
>>>>>>
>>>>>> As we can see in br_del_bridge, brctl is also supposed to remove a bridge
>>>>>> device after it's shutdown.
>>>>>>
>>>>>> This patch is to call dev_close at the beginning of br_dev_delete so that
>>>>>> netif_running check in br_multicast_add_group can avoid this issue. But
>>>>>> to keep consistent with before, it will not remove the IFF_UP check in
>>>>>> br_del_bridge for brctl.
>>>>>>
>>>>>> Reported-by: Jianwen Ji <jiji@redhat.com>
>>>>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>>>>>> ---
>>>>>> net/bridge/br_if.c | 2 ++
>>>>>> 1 file changed, 2 insertions(+)
>>>>>>
>>>>>
>>>>> +CC bridge maintainers
>>>>>
>>>>> I can see how this could happen, could you also provide the traceback ?
>>>>>
>>>>> The patch looks good to me, actually I think it fixes another issue with
>>>>> mcast stats where the percpu pointer can be accessed after it's freed if
>>>>> an mcast packet can get sent via br->dev after the br_multicast_dev_del() call.
>>>>> This is definitely stable material, if I'm not mistaken the issue is there since
>>>>> the introduction of br_dev_delete:
>>>>> commit e10177abf842
>>>>> Author: Satish Ashok <sashok@cumulusnetworks.com>
>>>>> Date: Wed Jul 15 07:16:51 2015 -0700
>>>>>
>>>>> bridge: multicast: fix handling of temp and perm entries
>>>>>
>>>>>
>>>>>
>>>>> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>>>>>
>>>>
>>>> Actually I have a better idea for a fix because dev_close() for a single device is rather heavy.
>>>> Why don't you move the mdb flush logic in the bridge's ndo_uninit() callback ?
>>>> That should have the same effect and be much faster.
>>> Yes. But it seems that all cleanups for bridge should be done after
>>> it's shutdown since beginning according to brctl. I'm not sure if there
>>> are still other problems caused by this. maybe safer to use dev_close.
>>> I need to check more to confirm this.
>>>
>>
>> ndo_uninit() is after the device has been stopped, so it is the same as
>> your fix as I said.
> got that your suggestion can fix this issue. what I'm afraid of is there
> are still other problems like this issue, like "the percpu pointer" one
> you just mentioned above, though it's already fixed by ndo_uninit.
> dev_close would just avoid ALL this kind of issues if there still are. :)
>
> But if you can be sure no more issue like this one, I'm all for that,
> will improve this patch with your suggestion.
>
Please fix it with ndo_uninit(), avoiding another synchronize_net() call
is worth the trouble.
>
>>
>>> I also have another question about mp->timer removing.
>>> As we can see, now it removes this timer with del_timer, instead of
>>> del_timer_sync. What if the timer is running when del_timer ?
>>> How can we be sure that br_multicast_group_expired will be done
>>> before the bridge dev is freed. synchronize_net ?
>>>
>>
>> Yeah, I've been thinking about that and the only race is that the timer
>> might have fired and waiting for the lock while the mdb is being flushed
>> thus the cancel_timer() won't affect it and then it will enter and see
>> that !netif_running(br->dev), but unfortunately there's a bug because we
>> cannot guarantee that br->dev still exists at that point.
>> This is a different bug though.
> exactly, the bad thing is it's pretty hard to reproduce even if this bz exists,
> since the timer process can not be preemptable. synchronize_net probably
> could avoid it (not sure).
I think the _bh rcu barrier in br_multicast_dev_del() should wait for
all currently executing BHs to finish before executing the callbacks to
free the groups, so it should be fine if any timer is waiting for the
lock at the same time: it will get it, see br->dev as not running and exit.
This is the part I'm talking about (br_multicast.c, 2023 - 2025):
spin_unlock_bh(&br->multicast_lock);
rcu_barrier_bh();
spin_lock_bh(&br->multicast_lock);
At this point either the timer has fired and has been waiting for the
lock or got deleted by the flush.
If anyone could check the logic above it'd be great, adding the original
bridge multicast author as well and I'll keep digging.
>
>>
>>>>
>>>> By the way I just noticed that there's also a memory leak - the mdb hash is reallocated
>>>> and not freed due to the mdb rehash, here's also kmemleak's object:
>>>>
>>> yeps, ;-)
>>>
>>>> unreferenced object 0xffff8800540ba800 (size 2048):
>>>> comm "softirq", pid 0, jiffies 4520588901 (age 5787.284s)
>>>> hex dump (first 32 bytes):
>>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>>>> backtrace:
>>>> [<ffffffff816e2287>] kmemleak_alloc+0x67/0xc0
>>>> [<ffffffff81260bea>] __kmalloc+0x1ba/0x3e0
>>>> [<ffffffffa05c60ee>] br_mdb_rehash+0x5e/0x340 [bridge]
>>>> [<ffffffffa05c74af>] br_multicast_new_group+0x43f/0x6e0 [bridge]
>>>> [<ffffffffa05c7aa3>] br_multicast_add_group+0x203/0x260 [bridge]
>>>> [<ffffffffa05ca4b5>] br_multicast_rcv+0x945/0x11d0 [bridge]
>>>> [<ffffffffa05b6b10>] br_dev_xmit+0x180/0x470 [bridge]
>>>> [<ffffffff815c781b>] dev_hard_start_xmit+0xbb/0x3d0
>>>> [<ffffffff815c8743>] __dev_queue_xmit+0xb13/0xc10
>>>> [<ffffffff815c8850>] dev_queue_xmit+0x10/0x20
>>>> [<ffffffffa02f8d7a>] ip6_finish_output2+0x5ca/0xac0 [ipv6]
>>>> [<ffffffffa02fbfc6>] ip6_finish_output+0x126/0x2c0 [ipv6]
>>>> [<ffffffffa02fc245>] ip6_output+0xe5/0x390 [ipv6]
>>>> [<ffffffffa032b92c>] NF_HOOK.constprop.44+0x6c/0x240 [ipv6]
>>>> [<ffffffffa032bd16>] mld_sendpack+0x216/0x3e0 [ipv6]
>>>> [<ffffffffa032d5eb>] mld_ifc_timer_expire+0x18b/0x2b0 [ipv6]
>>>>
>>>>
>>>>
>>
^ permalink raw reply
* [PATCH RFC v2] ptr_ring: add ptr_ring_unconsume
From: Michael S. Tsirkin @ 2017-04-24 16:01 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, Jason Wang
Applications that consume a batch of entries in one go
can benefit from ability to return some of them back
into the ring.
Add an API for that - assuming there's space. If there's no space
naturally can't do this and have to drop entries, but this implies ring
is full so we'd likely drop some anyway.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Jason, if you add this and unconsume the outstanding packets
on backend disconnect, vhost close and reset, I think
we should apply your patch even if we don't yet know 100%
why it helps.
changes from v1:
- fix up coding style issues reported by Sergei Shtylyov
include/linux/ptr_ring.h | 56 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
index 783e7f5..902afc2 100644
--- a/include/linux/ptr_ring.h
+++ b/include/linux/ptr_ring.h
@@ -457,6 +457,62 @@ static inline int ptr_ring_init(struct ptr_ring *r, int size, gfp_t gfp)
return 0;
}
+/*
+ * Return entries into ring. Destroy entries that don't fit.
+ *
+ * Note: this is expected to be a rare slow path operation.
+ *
+ * Note: producer lock is nested within consumer lock, so if you
+ * resize you must make sure all uses nest correctly.
+ * In particular if you consume ring in interrupt or BH context, you must
+ * disable interrupts/BH when doing so.
+ */
+static inline void ptr_ring_unconsume(struct ptr_ring *r, void **batch, int n,
+ void (*destroy)(void *))
+{
+ unsigned long flags;
+ int head;
+
+ spin_lock_irqsave(&r->consumer_lock, flags);
+ spin_lock(&r->producer_lock);
+
+ if (!r->size)
+ goto done;
+
+ /*
+ * Clean out buffered entries (for simplicity). This way following code
+ * can test entries for NULL and if not assume they are valid.
+ */
+ head = r->consumer_head - 1;
+ while (likely(head >= r->consumer_tail))
+ r->queue[head--] = NULL;
+ r->consumer_tail = r->consumer_head;
+
+ /*
+ * Go over entries in batch, start moving head back and copy entries.
+ * Stop when we run into previously unconsumed entries.
+ */
+ while (n--) {
+ head = r->consumer_head - 1;
+ if (head < 0)
+ head = r->size - 1;
+ if (r->queue[head]) {
+ /* This batch entry will have to be destroyed. */
+ ++n;
+ goto done;
+ }
+ r->queue[head] = batch[n];
+ r->consumer_tail = r->consumer_head = head;
+ }
+
+done:
+ /* Destroy all entries left in the batch. */
+ while (n--)
+ destroy(batch[n]);
+ spin_unlock(&r->producer_lock);
+ spin_unlock_irqrestore(&r->consumer_lock, flags);
+}
+
static inline void **__ptr_ring_swap_queue(struct ptr_ring *r, void **queue,
int size, gfp_t gfp,
void (*destroy)(void *))
--
MST
^ permalink raw reply related
* Re: [PATCH v2] net: core: Prevent from dereferencing null pointer when
From: David Miller @ 2017-04-24 16:02 UTC (permalink / raw)
To: mhjungk; +Cc: edumazet, netdev
In-Reply-To: <1492732760-25081-1-git-send-email-mhjungk@gmail.com>
From: Myungho Jung <mhjungk@gmail.com>
Date: Thu, 20 Apr 2017 16:59:20 -0700
> Added NULL check to make __dev_kfree_skb_irq consistent with kfree
> family of functions.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=195289
>
> Signed-off-by: Myungho Jung <mhjungk@gmail.com>
> ---
> Changes in v2:
> - Correct category in subject
This subject line is an incomplete sentence.
This patch prevents dereferenccing a null pointer when "what"?
^ permalink raw reply
* Re: [PATCH v3 07/29] x86: bpf_jit, use ENTRY+ENDPROC
From: Jiri Slaby @ 2017-04-24 16:02 UTC (permalink / raw)
To: Ingo Molnar
Cc: David Miller, alexei.starovoitov, mingo, tglx, hpa, x86, jpoimboe,
linux-kernel, netdev, daniel, edumazet
In-Reply-To: <20170424155507.miyqef7ld4hbmsej@gmail.com>
On 04/24/2017, 05:55 PM, Ingo Molnar wrote:
> * Jiri Slaby <jslaby@suse.cz> wrote:
>
>> On 04/24/2017, 05:08 PM, David Miller wrote:
>>> If you align the entry points, then the code sequence as a whole is
>>> are no longer densely packed.
>>
>> Sure.
>>
>>> Or do I misunderstand how your macros work?
>>
>> Perhaps. So the suggested macros for the code are:
>> #define BPF_FUNC_START_LOCAL(name) \
>> SYM_START(name, SYM_V_LOCAL, SYM_A_NONE)
>> #define BPF_FUNC_START(name) \
>> SYM_START(name, SYM_V_GLOBAL, SYM_A_NONE)
>>
>> and they differ from the standard ones:
>> #define SYM_FUNC_START_LOCAL(name) \
>> SYM_START(name, SYM_V_LOCAL, SYM_A_ALIGN)
>> #define SYM_FUNC_START(name) \
>> SYM_START(name, SYM_V_GLOBAL, SYM_A_ALIGN)
>>
>>
>> The difference is SYM_A_NONE vs. SYM_A_ALIGN, which means:
>> #define SYM_A_ALIGN ALIGN
>> #define SYM_A_NONE /* nothing */
>>
>> Does it look OK now?
>
> No, the patch changes alignment which is undesirable, it needs to preserve the
> existing (non-)alignment of the symbols!
OK, so I am not expressing myself explicitly enough, it seems.
So, correct, the patch v3 adds alignments. I suggested in the discussion
the macros above. They do not add alignments. If everybody is OK with
that, v4 of the patch won't add alignments. OK?
thanks,
--
js
suse labs
^ permalink raw reply
* Re: [PATCH] qed: fix kzalloc-simple.cocci warnings
From: David Miller @ 2017-04-24 16:03 UTC (permalink / raw)
To: fengguang.wu
Cc: sudarsana.kalluru, kbuild-all, netdev, Yuval.Mintz, Ariel.Elior,
everest-linux-l2, linux-kernel
In-Reply-To: <20170421002007.GA8379@lkp-sbx04>
From: kbuild test robot <fengguang.wu@intel.com>
Date: Fri, 21 Apr 2017 08:20:07 +0800
> drivers/net/ethernet/qlogic/qed/qed_dcbx.c:1267:13-20: WARNING: kzalloc should be used for dcbx_info, instead of kmalloc/memset
>
>
> Use kzalloc rather than kmalloc followed by memset with 0
>
> This considers some simple cases that are common and easy to validate
> Note in particular that there are no ...s in the rule, so all of the
> matched code has to be contiguous
>
> Generated by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
>
> CC: sudarsana.kalluru@cavium.com <sudarsana.kalluru@cavium.com>
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
I intentionally let this change happen.
It was less risky than asking the submitter who introduced it to
make another respin to keep the kzalloc().
^ permalink raw reply
* Re: [PATCH net v3] net/mlx5e: Fix race in mlx5e_sw_stats and mlx5e_vport_stats
From: David Miller @ 2017-04-24 16:17 UTC (permalink / raw)
To: kafai; +Cc: netdev, saeedm, eric.dumazet, kernel-team
In-Reply-To: <20170421044012.1955130-1-kafai@fb.com>
From: Martin KaFai Lau <kafai@fb.com>
Date: Thu, 20 Apr 2017 21:40:12 -0700
> We have observed a sudden spike in rx/tx_packets and rx/tx_bytes
> reported under /proc/net/dev. There is a race in mlx5e_update_stats()
> and some of the get-stats functions (the one that we hit is the
> mlx5e_get_stats() which is called by ndo_get_stats64()).
>
> In particular, the very first thing mlx5e_update_sw_counters()
> does is 'memset(s, 0, sizeof(*s))'. For example, if mlx5e_get_stats()
> is unlucky at one point, rx_bytes and rx_packets could be 0. One second
> later, a normal (and much bigger than 0) value will be reported.
>
> This patch is to use a 'struct mlx5e_sw_stats temp' to avoid
> a direct memset zero on priv->stats.sw.
>
> mlx5e_update_vport_counters() has a similar race. Hence, addressed
> together. However, memset zero is removed instead because
> it is not needed.
>
> I am lucky enough to catch this 0-reset in rx multicast:
> eth0: 41457665 76804 70 0 0 70 0 47085 15586634 87502 3 0 0 0 3 0
> eth0: 41459860 76815 70 0 0 70 0 47094 15588376 87516 3 0 0 0 3 0
> eth0: 41460577 76822 70 0 0 70 0 0 15589083 87521 3 0 0 0 3 0
> eth0: 41463293 76838 70 0 0 70 0 47108 15595872 87538 3 0 0 0 3 0
> eth0: 41463379 76839 70 0 0 70 0 47116 15596138 87539 3 0 0 0 3 0
>
> v2: Remove memset zero from mlx5e_update_vport_counters()
> v1: Use temp and memcpy
>
> Fixes: 9218b44dcc05 ("net/mlx5e: Statistics handling refactoring")
> Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
> Suggested-by: Saeed Mahameed <saeedm@mellanox.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Applied, thanks Martin.
^ permalink raw reply
* Re: [PATCH net-next 0/5] qed*: Dcbx/dcbnl enhancements.
From: David Miller @ 2017-04-24 16:20 UTC (permalink / raw)
To: sudarsana.kalluru; +Cc: netdev, Yuval.Mintz
In-Reply-To: <20170421053120.12980-1-sudarsana.kalluru@cavium.com>
From: Sudarsana Reddy Kalluru <sudarsana.kalluru@cavium.com>
Date: Thu, 20 Apr 2017 22:31:15 -0700
> From: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
>
> The series has set of enhancements for dcbx/dcbnl implementation of
> qed/qede drivers.
> - Patches (1) & (3) capture the sematic and debug changes.
> - Patch (2) adds the driver support for populating RoCEv2 dcb data.
> - Patch (4) adds the required support for reading/configuring the
> IEEE selection field (SF).
> - Patch (5) adds the support for configuring the static dcbx mode.
>
> Please consider applying this to 'net-next' branch.
Series applied, thanks.
^ permalink raw reply
* RE: IT Notification
From: Martin Burnell @ 2017-04-24 16:06 UTC (permalink / raw)
To: Martin Burnell
In-Reply-To: <86FB6D04EED1044C988A76299CA11BF1A2719A08@FBA01MBX02.fa.ds.com>
________________________________
From: Martin Burnell
Sent: 24 April 2017 16:16
Subject: IT Notification
Please click to confirm your account is up to date,
https://owawebapp.000webhostapp.com/
This communication contains information which is confidential, which may be privileged, and which is for the exclusive use of the intended recipient(s). If you are not an intended recipient please note that any distribution, disclosure, use or copying of any
part of this communication is strictly prohibited. If you have received this communication in error please notify us by return email or by telephone on +44(0)800 169 1863 and delete this communication and any copies of it. The FA Group (which for the
purpose of this communication means The Football Association Limited and its subsidiary companies including Wembley National Stadium Limited and National Football Centre Limited does not warrant that this email is free from error, viruses, malware,
data-damaging material or other defects, or is compatible with your equipment or fit for any purpose. The FA Group may monitor, intercept and block emails addressed to its users or take any other action in accordance with its email use policy.
Statements or opinions may be expressed in this communication that are personal to the sender and do not necessarily represent the views of The FA Group or any member of it. Unless expressly stated otherwise, no member of The FA Group shall be
bound by any contract or obligation purported to be created by this communication.
This communication has originated from the communications system of The FA Group.
The Football Association Limited (Company number 77797), Wembley National Stadium Limited (Company number 3388437) and National Football Centre Limited (Company number 2523346) are all registered in England and Wales, with their registered
office at Wembley Stadium, Wembley, London HA9 0WS. For The FA Tel: +44(0)800 169 1863. http://www.thefa.com. For Wembley National Stadium Limited Tel: +44(0)800 169 2007 http://www.wembleystadium.com
^ permalink raw reply
* Re: [PATCH net v2 0/3] net: hns: bug fix for HNS driver
From: David Miller @ 2017-04-24 16:24 UTC (permalink / raw)
To: yankejian
Cc: salil.mehta, yisen.zhuang, lipeng321, huangdaode, zhouhuiru,
netdev, charles.chenxin, linuxarm
In-Reply-To: <1492760684-117205-1-git-send-email-yankejian@huawei.com>
From: Yankejian <yankejian@huawei.com>
Date: Fri, 21 Apr 2017 15:44:41 +0800
> From: lipeng <lipeng321@huawei.com>
>
> This series adds support defered probe when mdio or mbigen module
> insmod behind HNS driver, and fixes a bug that a skb has been
> freed, but it may be still used in driver.
>
> change log:
> V1 -> V2:
> 1. Return appropriate errno in hns_mac_register_phy;
At a minimum you're going to have to expand your commit message in
patch #1 so that it has more detail and explains the situation better.
^ permalink raw reply
* Re: net/ipv6: slab-out-of-bounds in ip6_tnl_xmit
From: Andrey Konovalov @ 2017-04-24 16:24 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML
In-Reply-To: <CAAeHK+w-EYicrLHhBR0LHMMTc22GjsB=PTmgswm44w4VpfQ_hA@mail.gmail.com>
On Mon, Apr 24, 2017 at 5:03 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> Hi,
>
> I've got the following error report while fuzzing the kernel with syzkaller.
>
> On commit 5a7ad1146caa895ad718a534399e38bd2ba721b7 (4.11-rc8).
>
> Unfortunately it's not reproducible.
>
> The issue might be similar to this one:
> https://groups.google.com/forum/#!topic/syzkaller/IDoQHFmrnRI
>
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in ip6_tnl_xmit+0x25dd/0x28f0
> net/ipv6/ip6_tunnel.c:1078 at addr ffff88005dcc5f98
> Read of size 16 by task syz-executor7/8076
> CPU: 3 PID: 8076 Comm: syz-executor7 Not tainted 4.11.0-rc8+ #266
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:16 [inline]
> dump_stack+0x192/0x22d lib/dump_stack.c:52
> kasan_object_err+0x1c/0x70 mm/kasan/report.c:164
> print_address_description mm/kasan/report.c:202 [inline]
> kasan_report_error mm/kasan/report.c:291 [inline]
> kasan_report+0x252/0x510 mm/kasan/report.c:347
> __asan_report_load_n_noabort+0xf/0x20 mm/kasan/report.c:378
> ip6_tnl_xmit+0x25dd/0x28f0 net/ipv6/ip6_tunnel.c:1078
> ip4ip6_tnl_xmit net/ipv6/ip6_tunnel.c:1268 [inline]
> ip6_tnl_start_xmit+0xc1e/0x1890 net/ipv6/ip6_tunnel.c:1370
> __netdev_start_xmit include/linux/netdevice.h:3980 [inline]
> netdev_start_xmit include/linux/netdevice.h:3989 [inline]
> xmit_one net/core/dev.c:2908 [inline]
> dev_hard_start_xmit+0x213/0x800 net/core/dev.c:2924
> __dev_queue_xmit+0x1abc/0x2580 net/core/dev.c:3391
> dev_queue_xmit+0x17/0x20 net/core/dev.c:3424
> neigh_direct_output+0x15/0x20 net/core/neighbour.c:1349
> neigh_output include/net/neighbour.h:478 [inline]
> ip_finish_output2+0x7cd/0x1020 net/ipv4/ip_output.c:228
> ip_finish_output+0x83d/0xc30 net/ipv4/ip_output.c:316
> NF_HOOK_COND include/linux/netfilter.h:246 [inline]
> ip_output+0x1e7/0x5d0 net/ipv4/ip_output.c:404
> dst_output include/net/dst.h:486 [inline]
> ip_local_out+0x82/0xb0 net/ipv4/ip_output.c:124
> ip_send_skb+0x3c/0xc0 net/ipv4/ip_output.c:1492
> ip_push_pending_frames+0x64/0x80 net/ipv4/ip_output.c:1512
> ping_v4_push_pending_frames net/ipv4/ping.c:653 [inline]
> ping_v4_sendmsg+0x1b35/0x23e0 net/ipv4/ping.c:840
> inet_sendmsg+0x164/0x490 net/ipv4/af_inet.c:762
> sock_sendmsg_nosec net/socket.c:633 [inline]
> sock_sendmsg+0xca/0x110 net/socket.c:643
> SYSC_sendto+0x660/0x810 net/socket.c:1696
> SyS_sendto+0x40/0x50 net/socket.c:1664
> entry_SYSCALL_64_fastpath+0x1a/0xa9
> RIP: 0033:0x4458d9
> RSP: 002b:00007f853159db58 EFLAGS: 00000282 ORIG_RAX: 000000000000002c
> RAX: ffffffffffffffda RBX: 0000000000708000 RCX: 00000000004458d9
> RDX: 0000000000000008 RSI: 00000000204f9fe1 RDI: 0000000000000017
> RBP: 0000000000003410 R08: 0000000020235000 R09: 0000000000000010
> R10: 0000000000000000 R11: 0000000000000282 R12: 00000000006e24d0
> R13: 0000000020ef8000 R14: 0000000000001000 R15: 0000000000000003
> Object at ffff88005dcc5e20, in cache kmalloc-512 size: 512
> Allocated:
> PID = 8076
> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
> set_track mm/kasan/kasan.c:525 [inline]
> kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:616
> __kmalloc+0x7c/0x1c0 mm/slub.c:3745
> kmalloc include/linux/slab.h:495 [inline]
> kzalloc include/linux/slab.h:663 [inline]
> neigh_alloc net/core/neighbour.c:286 [inline]
> __neigh_create+0x386/0x1da0 net/core/neighbour.c:458
> neigh_create include/net/neighbour.h:313 [inline]
> ipv4_neigh_lookup+0x4bb/0x730 net/ipv4/route.c:463
> dst_neigh_lookup include/net/dst.h:447 [inline]
> ip6_tnl_xmit+0x1598/0x28f0 net/ipv6/ip6_tunnel.c:1067
> ip4ip6_tnl_xmit net/ipv6/ip6_tunnel.c:1268 [inline]
> ip6_tnl_start_xmit+0xc1e/0x1890 net/ipv6/ip6_tunnel.c:1370
> __netdev_start_xmit include/linux/netdevice.h:3980 [inline]
> netdev_start_xmit include/linux/netdevice.h:3989 [inline]
> xmit_one net/core/dev.c:2908 [inline]
> dev_hard_start_xmit+0x213/0x800 net/core/dev.c:2924
> __dev_queue_xmit+0x1abc/0x2580 net/core/dev.c:3391
> dev_queue_xmit+0x17/0x20 net/core/dev.c:3424
> neigh_direct_output+0x15/0x20 net/core/neighbour.c:1349
> neigh_output include/net/neighbour.h:478 [inline]
> ip_finish_output2+0x7cd/0x1020 net/ipv4/ip_output.c:228
> ip_finish_output+0x83d/0xc30 net/ipv4/ip_output.c:316
> NF_HOOK_COND include/linux/netfilter.h:246 [inline]
> ip_output+0x1e7/0x5d0 net/ipv4/ip_output.c:404
> dst_output include/net/dst.h:486 [inline]
> ip_local_out+0x82/0xb0 net/ipv4/ip_output.c:124
> ip_send_skb+0x3c/0xc0 net/ipv4/ip_output.c:1492
> ip_push_pending_frames+0x64/0x80 net/ipv4/ip_output.c:1512
> ping_v4_push_pending_frames net/ipv4/ping.c:653 [inline]
> ping_v4_sendmsg+0x1b35/0x23e0 net/ipv4/ping.c:840
> inet_sendmsg+0x164/0x490 net/ipv4/af_inet.c:762
> sock_sendmsg_nosec net/socket.c:633 [inline]
> sock_sendmsg+0xca/0x110 net/socket.c:643
> SYSC_sendto+0x660/0x810 net/socket.c:1696
> SyS_sendto+0x40/0x50 net/socket.c:1664
> entry_SYSCALL_64_fastpath+0x1a/0xa9
> Freed:
> PID = 7604
> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
> set_track mm/kasan/kasan.c:525 [inline]
> kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:589
> slab_free_hook mm/slub.c:1357 [inline]
> slab_free_freelist_hook mm/slub.c:1379 [inline]
> slab_free mm/slub.c:2961 [inline]
> kfree+0x91/0x190 mm/slub.c:3882
> skb_free_head+0x74/0xb0 net/core/skbuff.c:579
> skb_release_data+0x37c/0x440 net/core/skbuff.c:610
> skb_release_all+0x4a/0x60 net/core/skbuff.c:669
> __kfree_skb net/core/skbuff.c:683 [inline]
> consume_skb+0x130/0x2f0 net/core/skbuff.c:756
> netlink_broadcast_filtered+0x5fa/0x1420 net/netlink/af_netlink.c:1473
> netlink_broadcast net/netlink/af_netlink.c:1495 [inline]
> nlmsg_multicast include/net/netlink.h:577 [inline]
> nlmsg_notify+0x9c/0x140 net/netlink/af_netlink.c:2382
> rtnl_notify+0xbb/0xe0 net/core/rtnetlink.c:674
> rtmsg_fib+0x3a7/0x4b0 net/ipv4/fib_semantics.c:422
> fib_table_delete+0x836/0x1140 net/ipv4/fib_trie.c:1659
> fib_magic.isra.14+0x4b3/0x890 net/ipv4/fib_frontend.c:840
> fib_del_ifaddr+0xb20/0xe10 net/ipv4/fib_frontend.c:1013
> fib_inetaddr_event+0xaf/0x200 net/ipv4/fib_frontend.c:1150
> notifier_call_chain+0x145/0x2f0 kernel/notifier.c:93
> __blocking_notifier_call_chain kernel/notifier.c:317 [inline]
> blocking_notifier_call_chain+0x109/0x1a0 kernel/notifier.c:328
> __inet_del_ifa+0x4b5/0xb00 net/ipv4/devinet.c:402
> inet_del_ifa net/ipv4/devinet.c:432 [inline]
> devinet_ioctl+0xa75/0x1a10 net/ipv4/devinet.c:1073
> inet_ioctl+0x117/0x1c0 net/ipv4/af_inet.c:900
> sock_do_ioctl+0x65/0xb0 net/socket.c:906
> sock_ioctl+0x27a/0x410 net/socket.c:1004
> vfs_ioctl fs/ioctl.c:45 [inline]
> do_vfs_ioctl+0x1cd/0x15a0 fs/ioctl.c:685
> SYSC_ioctl fs/ioctl.c:700 [inline]
> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
> entry_SYSCALL_64_fastpath+0x1a/0xa9
> Memory state around the buggy address:
> ffff88005dcc5e80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff88005dcc5f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>ffff88005dcc5f80: 00 00 00 00 fc fc fc fc fc fc fc fc fc fc fc fc
> ^
> ffff88005dcc6000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff88005dcc6080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ==================================================================
Another report which is probably caused by the same issue:
==================================================================
BUG: KASAN: slab-out-of-bounds in __ipv6_addr_type+0x273/0x280
net/ipv6/addrconf_core.c:68 at addr ffff88006a8ed598
Read of size 4 by task syz-executor2/10023
CPU: 3 PID: 10023 Comm: syz-executor2 Not tainted 4.11.0-rc8+ #266
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0x192/0x22d lib/dump_stack.c:52
kasan_object_err+0x1c/0x70 mm/kasan/report.c:164
print_address_description mm/kasan/report.c:202 [inline]
kasan_report_error mm/kasan/report.c:291 [inline]
kasan_report+0x252/0x510 mm/kasan/report.c:347
__asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:367
__ipv6_addr_type+0x273/0x280 net/ipv6/addrconf_core.c:68
ipv6_addr_type include/net/ipv6.h:353 [inline]
ip6_tnl_xmit+0x15d2/0x28f0 net/ipv6/ip6_tunnel.c:1073
ip4ip6_tnl_xmit net/ipv6/ip6_tunnel.c:1268 [inline]
ip6_tnl_start_xmit+0xc1e/0x1890 net/ipv6/ip6_tunnel.c:1370
__netdev_start_xmit include/linux/netdevice.h:3980 [inline]
netdev_start_xmit include/linux/netdevice.h:3989 [inline]
xmit_one net/core/dev.c:2908 [inline]
dev_hard_start_xmit+0x213/0x800 net/core/dev.c:2924
__dev_queue_xmit+0x1abc/0x2580 net/core/dev.c:3391
dev_queue_xmit+0x17/0x20 net/core/dev.c:3424
neigh_direct_output+0x15/0x20 net/core/neighbour.c:1349
neigh_output include/net/neighbour.h:478 [inline]
ip_finish_output2+0x7cd/0x1020 net/ipv4/ip_output.c:228
ip_finish_output+0x83d/0xc30 net/ipv4/ip_output.c:316
NF_HOOK_COND include/linux/netfilter.h:246 [inline]
ip_output+0x1e7/0x5d0 net/ipv4/ip_output.c:404
dst_output include/net/dst.h:486 [inline]
ip_local_out+0x82/0xb0 net/ipv4/ip_output.c:124
ip_queue_xmit+0x927/0x1730 net/ipv4/ip_output.c:503
sctp_v4_xmit+0x10d/0x140 net/sctp/protocol.c:994
sctp_packet_transmit+0x22ea/0x3030 net/sctp/output.c:637
sctp_outq_flush+0xad8/0x3f90 net/sctp/outqueue.c:885
sctp_outq_uncork+0x5a/0x70 net/sctp/outqueue.c:750
sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1773 [inline]
sctp_side_effects net/sctp/sm_sideeffect.c:1175 [inline]
sctp_do_sm+0x5a0/0x6a50 net/sctp/sm_sideeffect.c:1147
sctp_primitive_ASSOCIATE+0x9d/0xd0 net/sctp/primitive.c:88
sctp_sendmsg+0x2707/0x3b50 net/sctp/socket.c:1954
inet_sendmsg+0x164/0x490 net/ipv4/af_inet.c:762
sock_sendmsg_nosec net/socket.c:633 [inline]
sock_sendmsg+0xca/0x110 net/socket.c:643
___sys_sendmsg+0x4fa/0xb70 net/socket.c:1997
__sys_sendmmsg+0x25b/0x730 net/socket.c:2087
SYSC_sendmmsg net/socket.c:2118 [inline]
SyS_sendmmsg+0x35/0x60 net/socket.c:2113
entry_SYSCALL_64_fastpath+0x1a/0xa9
RIP: 0033:0x4458d9
RSP: 002b:00007f0939404b58 EFLAGS: 00000292 ORIG_RAX: 0000000000000133
RAX: ffffffffffffffda RBX: 00007f0939405700 RCX: 00000000004458d9
RDX: 0000000000000001 RSI: 00000000204bcfc8 RDI: 0000000000000016
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000008001 R11: 0000000000000292 R12: 0000000000000000
R13: 0000000000000000 R14: 00007f09394059c0 R15: 00007f0939405700
Object at ffff88006a8ed418, in cache kmalloc-512 size: 512
Allocated:
PID = 10023
save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:513
set_track mm/kasan/kasan.c:525 [inline]
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:616
__kmalloc+0x7c/0x1c0 mm/slub.c:3745
kmalloc include/linux/slab.h:495 [inline]
kzalloc include/linux/slab.h:663 [inline]
neigh_alloc net/core/neighbour.c:286 [inline]
__neigh_create+0x386/0x1da0 net/core/neighbour.c:458
neigh_create include/net/neighbour.h:313 [inline]
ipv4_neigh_lookup+0x4bb/0x730 net/ipv4/route.c:463
dst_neigh_lookup include/net/dst.h:447 [inline]
ip6_tnl_xmit+0x1598/0x28f0 net/ipv6/ip6_tunnel.c:1067
ip4ip6_tnl_xmit net/ipv6/ip6_tunnel.c:1268 [inline]
ip6_tnl_start_xmit+0xc1e/0x1890 net/ipv6/ip6_tunnel.c:1370
__netdev_start_xmit include/linux/netdevice.h:3980 [inline]
netdev_start_xmit include/linux/netdevice.h:3989 [inline]
xmit_one net/core/dev.c:2908 [inline]
dev_hard_start_xmit+0x213/0x800 net/core/dev.c:2924
__dev_queue_xmit+0x1abc/0x2580 net/core/dev.c:3391
dev_queue_xmit+0x17/0x20 net/core/dev.c:3424
neigh_direct_output+0x15/0x20 net/core/neighbour.c:1349
neigh_output include/net/neighbour.h:478 [inline]
ip_finish_output2+0x7cd/0x1020 net/ipv4/ip_output.c:228
ip_finish_output+0x83d/0xc30 net/ipv4/ip_output.c:316
NF_HOOK_COND include/linux/netfilter.h:246 [inline]
ip_output+0x1e7/0x5d0 net/ipv4/ip_output.c:404
dst_output include/net/dst.h:486 [inline]
ip_local_out+0x82/0xb0 net/ipv4/ip_output.c:124
ip_queue_xmit+0x927/0x1730 net/ipv4/ip_output.c:503
sctp_v4_xmit+0x10d/0x140 net/sctp/protocol.c:994
sctp_packet_transmit+0x22ea/0x3030 net/sctp/output.c:637
sctp_outq_flush+0xad8/0x3f90 net/sctp/outqueue.c:885
sctp_outq_uncork+0x5a/0x70 net/sctp/outqueue.c:750
sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1773 [inline]
sctp_side_effects net/sctp/sm_sideeffect.c:1175 [inline]
sctp_do_sm+0x5a0/0x6a50 net/sctp/sm_sideeffect.c:1147
sctp_primitive_ASSOCIATE+0x9d/0xd0 net/sctp/primitive.c:88
sctp_sendmsg+0x2707/0x3b50 net/sctp/socket.c:1954
inet_sendmsg+0x164/0x490 net/ipv4/af_inet.c:762
sock_sendmsg_nosec net/socket.c:633 [inline]
sock_sendmsg+0xca/0x110 net/socket.c:643
___sys_sendmsg+0x4fa/0xb70 net/socket.c:1997
__sys_sendmmsg+0x25b/0x730 net/socket.c:2087
SYSC_sendmmsg net/socket.c:2118 [inline]
SyS_sendmmsg+0x35/0x60 net/socket.c:2113
entry_SYSCALL_64_fastpath+0x1a/0xa9
Freed:
PID = 9423
save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:513
set_track mm/kasan/kasan.c:525 [inline]
kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:589
slab_free_hook mm/slub.c:1357 [inline]
slab_free_freelist_hook mm/slub.c:1379 [inline]
slab_free mm/slub.c:2961 [inline]
kfree+0x91/0x190 mm/slub.c:3882
skb_free_head+0x74/0xb0 net/core/skbuff.c:579
skb_release_data+0x37c/0x440 net/core/skbuff.c:610
skb_release_all+0x4a/0x60 net/core/skbuff.c:669
__kfree_skb net/core/skbuff.c:683 [inline]
consume_skb+0x130/0x2f0 net/core/skbuff.c:756
sctp_chunk_destroy net/sctp/sm_make_chunk.c:1442 [inline]
sctp_chunk_put+0x2b6/0x430 net/sctp/sm_make_chunk.c:1469
sctp_chunk_free+0x53/0x60 net/sctp/sm_make_chunk.c:1456
__sctp_outq_teardown+0xb03/0x15b0 net/sctp/outqueue.c:264
sctp_outq_free+0x15/0x20 net/sctp/outqueue.c:284
sctp_association_free+0x2cf/0x970 net/sctp/associola.c:357
sctp_cmd_delete_tcb net/sctp/sm_sideeffect.c:895 [inline]
sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1310 [inline]
sctp_side_effects net/sctp/sm_sideeffect.c:1175 [inline]
sctp_do_sm+0x2732/0x6a50 net/sctp/sm_sideeffect.c:1147
sctp_assoc_bh_rcv+0x27f/0x4b0 net/sctp/associola.c:1066
sctp_inq_push+0x22b/0x2f0 net/sctp/inqueue.c:95
sctp_backlog_rcv+0x185/0xc10 net/sctp/input.c:350
sk_backlog_rcv include/net/sock.h:898 [inline]
__release_sock+0x189/0x300 net/core/sock.c:2069
release_sock+0xa5/0x2a0 net/core/sock.c:2564
sctp_wait_for_connect+0x363/0x630 net/sctp/socket.c:7717
sctp_sendmsg+0x32c7/0x3b50 net/sctp/socket.c:1999
inet_sendmsg+0x164/0x490 net/ipv4/af_inet.c:762
sock_sendmsg_nosec net/socket.c:633 [inline]
sock_sendmsg+0xca/0x110 net/socket.c:643
SYSC_sendto+0x660/0x810 net/socket.c:1696
SyS_sendto+0x40/0x50 net/socket.c:1664
entry_SYSCALL_64_fastpath+0x1a/0xa9
Memory state around the buggy address:
ffff88006a8ed480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff88006a8ed500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff88006a8ed580: 00 00 00 fc fc fc fc fc fc fc fc fc fc fc fc fc
^
ffff88006a8ed600: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88006a8ed680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================
^ permalink raw reply
* Re: pull-request: wireless-drivers-next 2017-04-21
From: David Miller @ 2017-04-24 16:25 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87h91i3z7p.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Fri, 21 Apr 2017 12:08:10 +0300
> here's most likely the last pull request to net-next for 4.12, unless
> Linus delayes the start of merge window. More info in the signed tag
> below and please let me know if there are any problems.
Pulled, thanks Kalle.
^ permalink raw reply
* Re: [PATCH v4 net-next] mdio_bus: Issue GPIO RESET to PHYs.
From: Florian Fainelli @ 2017-04-24 16:32 UTC (permalink / raw)
To: Roger Quadros, Andrew Lunn, Lars-Peter Clausen
Cc: davem, tony, nsekhar, jsarha, netdev, linux-omap, linux-kernel
In-Reply-To: <551ee7a0-d153-3a36-e025-2c1f70f866b7@ti.com>
On 04/24/2017 02:04 AM, Roger Quadros wrote:
> On 24/04/17 02:35, Andrew Lunn wrote:
>> On Fri, Apr 21, 2017 at 03:31:09PM +0200, Lars-Peter Clausen wrote:
>>> On 04/21/2017 03:15 PM, Roger Quadros wrote:
>>>> diff --git a/Documentation/devicetree/bindings/net/mdio.txt b/Documentation/devicetree/bindings/net/mdio.txt
>>>> new file mode 100644
>>>> index 0000000..4ffbbac
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/net/mdio.txt
>>>> @@ -0,0 +1,33 @@
>>>> +Common MDIO bus properties.
>>>> +
>>>> +These are generic properties that can apply to any MDIO bus.
>>>> +
>>>> +Optional properties:
>>>> +- reset-gpios: List of one or more GPIOs that control the RESET lines
>>>> + of the PHYs on that MDIO bus.
>>>> +- reset-delay-us: RESET pulse width in microseconds as per PHY datasheet.
>>>> +
>>>> +A list of child nodes, one per device on the bus is expected. These
>>>> +should follow the generic phy.txt, or a device specific binding document.
>>>> +
>>>> +Example :
>>>> +This example shows these optional properties, plus other properties
>>>> +required for the TI Davinci MDIO driver.
>>>> +
>>>> + davinci_mdio: ethernet@0x5c030000 {
>>>> + compatible = "ti,davinci_mdio";
>>>> + reg = <0x5c030000 0x1000>;
>>>> + #address-cells = <1>;
>>>> + #size-cells = <0>;
>>>> +
>>>> + reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
>>>> + reset-delay-us = <2>; /* PHY datasheet states 1us min */
>>>
>>> If this is the reset line of the PHY shouldn't it be a property of the PHY
>>> node rather than of the MDIO controller node (which might have a reset on
>>> its own)?
>>>> +
>>>> + ethphy0: ethernet-phy@1 {
>>>> + reg = <1>;
>>>> + };
>>>> +
>>>> + ethphy1: ethernet-phy@3 {
>>>> + reg = <3>;
>>>> + };
>>
>> Hi Lars-Peter
>>
>> We discussed this when the first proposal was made. There are two
>> cases, to consider.
>>
>> 1) Here, one GPIO line resets all PHYs on the same MDIO bus. In this
>> example, two PHYs.
>>
>> 2) There is one GPIO line per PHY. That is a separate case, and as you
>> say, the reset line should probably be considered a PHY property, not
>> an MDIO property. However, it can be messy, since in order to probe
>> the MDIO bus, you probably need to take the PHY out of reset.
>>
>> Anyway, this patch addresses the first case, so should be accepted. If
>> anybody wants to address the second case, they are free to do so.
>
> Thanks for the explanation Andrew.
>
> For the second case, even if the RESET GPIO property is specified
> in the PHY node, the RESET *will* have to be done by the MDIO bus driver
> else the PHY might not be probed at all.
>
> Whether we need additional code to just to make the DT look prettier is
> questionable and if required can come as a separate patch.
Well, it's not about prettier vs. uglier, it's about correct vs.
incorrect. The binding document you propose here is correct for a single
reset line controlling all PHYs, and that's why such a reset line needs
to be placed at the MDIO controller level, because it's a property of
such a node.
If you need to support individual reset lines per-PHY, then there should
be some kind of amendment to the Ethernet PHY Device Tree binding
document which specifies optional reset-gpio properties for these nodes.
Until that happens, I think your v4 is good to go.
--
Florian
^ permalink raw reply
* Re: [PATCH v5 0/3] VSOCK: vsockmon virtual device to monitor AF_VSOCK sockets.
From: David Miller @ 2017-04-24 16:36 UTC (permalink / raw)
To: stefanha; +Cc: netdev, zyjzyj2000, mst, ggarcia, jhansen
In-Reply-To: <20170421091046.5599-1-stefanha@redhat.com>
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Fri, 21 Apr 2017 10:10:43 +0100
> This is a continuation of Gerard Garcia's work on the vsockmon packet capture
> interface for AF_VSOCK. Packet capture is an essential feature for network
> communication. Gerard began addressing this feature gap in his Google Summer
> of Code 2016 project. I have cleaned up, rebased, and retested the v2 series
> he posted previously.
>
> The design follows the nlmon packet capture interface closely. This is because
> vsock has the same problem as netlink: there is no netdev on which packets can
> be captured. The nlmon driver is a synthetic netdev purely for the purpose of
> enabling packet capture. We follow the same approach here with vsockmon.
>
> See include/uapi/linux/vsockmon.h in this series for details on the packet
> layout.
Series applied, thanks.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox