Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] tc: f_u32: Fill in 'linkid' provided by user
From: Stephen Hemminger @ 2016-10-10  1:58 UTC (permalink / raw)
  To: Sushma Sitaram; +Cc: netdev, john.r.fastabend
In-Reply-To: <20160928183016.23280.39364.stgit@localhost.localdomain>

On Wed, 28 Sep 2016 11:30:16 -0700
Sushma Sitaram <sushma.sitaram@intel.com> wrote:

> Currently, 'linkid' input by the user is parsed but 'handle' is appended to the netlink message.
> 
> # tc filter add dev enp1s0f1 protocol ip parent ffff: prio 99 u32 ht 800: \
> 	order 1 link 1: offset at 0 mask 0f00 shift 6 plus 0 eat match ip \
> 	protocol 6 ff
> 
> resulted in:
> filter protocol ip pref 99 u32 fh 800::1 order 1 key ht 800 bkt 0
>   match 00060000/00ff0000 at 8
>     offset 0f00>>6 at 0  eat
> 
> This patch results in:
> filter protocol ip pref 99 u32 fh 800::1 order 1 key ht 800 bkt 0 link 1:
>   match 00060000/00ff0000 at 8
>     offset 0f00>>6 at 0  eat
> 
> 
> Signed-off-by Sushma Sitaram: Sushma Sitaram <sushma.sitaram@intel.com>

Applied (for 4.8).

^ permalink raw reply

* Re: [iproute2] bridge: Fix garbled json output seen if a vlan filter is specified
From: Stephen Hemminger @ 2016-10-10  1:58 UTC (permalink / raw)
  To: Anuradha Karuppiah; +Cc: netdev, roopa
In-Reply-To: <1475858418-1618-1-git-send-email-anuradhak@cumulusnetworks.com>

On Fri,  7 Oct 2016 09:40:18 -0700
Anuradha Karuppiah <anuradhak@cumulusnetworks.com> wrote:

> From: anuradhak <anuradhak@cumulusnetworks.com>
> 
> json objects were started but not completed if the fdb vlan did not
> match the specified filter vlan.
> 
> Sample output:
> $ bridge -j fdb show vlan 111
> [{
>         "mac": "44:38:39:00:69:88",
>         "dev": "br0",
>         "vlan": 111,
>         "master": "br0",
>         "state": "permanent"
>     }
> ]
> $ bridge -j fdb show vlan 100
> []
> $
> 
> Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>

Applied for 4.8 version

^ permalink raw reply

* Re: [PATCH iproute2] fix netlink message length checks
From: Stephen Hemminger @ 2016-10-10  1:59 UTC (permalink / raw)
  To: Igor Ryzhov; +Cc: netdev
In-Reply-To: <1475576215-9829-1-git-send-email-iryzhov@nfware.com>

On Tue,  4 Oct 2016 13:16:55 +0300
Igor Ryzhov <iryzhov@nfware.com> wrote:

> Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>

Makes sense applied, I wonder why one of the static checkers didn't see this.

^ permalink raw reply

* [ANNOUNCE] iproute 4.8
From: Stephen Hemminger @ 2016-10-10  2:05 UTC (permalink / raw)
  To: netdev, linux-kernel

Release of iproute2 for Linux 4.8, slightly late because of netdev.

Update to iproute2 utility to support new features in Linux 4.8.
Includes support for MACSEC and ILA.
Plus the usual array of documentation and minor fixes.

Source:
  http://www.kernel.org/pub/linux/utils/net/iproute2/iproute2-4.8.0.tar.gz

Repository:
  git://git.kernel.org/pub/scm/linux/kernel/git/shemminger/iproute2.git

Report problems (or enhancements) to the netdev@vger.kernel.org mailing list.

---
Andrey Jr. Melnikov (1):
      iproute: disallow ip rule del without parameters

David Ahern (1):
      ip rule: Add support for l3mdev rules

Davide Caratti (5):
      macsec: fix input of 'port', improve documentation of 'address'
      man: ip.8: add missing 'macsec' item to OBJECT list
      macsec: fix byte ordering on input/display of 'sci'
      tc: don't accept qdisc 'handle' greater than ffff
      macsec: fix input range of 'icvlen' parameter

Eric Dumazet (1):
      ip: report IFLA_GSO_MAX_SIZE and IFLA_GSO_MAX_SEGS

Gustavo Zacarias (1):
      ss: fix build with musl libc

Hangbin Liu (5):
      nstat: add sctp snmp support
      gitignore: Ignore 'tags' file generated by ctags
      ip route: check ftell, fseek return value
      misc/ss: tcp cwnd should be unsigned
      ip: Use specific slave id

Hannes Frederic Sowa (1):
      iptuntap: show processes using tuntap interface

Igor Ryzhov (1):
      fix netlink message length checks

Iskren Chernev (1):
      iproute: fix documentation for ip rule scan order

Jamal Hadi Salim (2):
      actions: skbedit add support for mod-ing skb pkt_type
      tc classifiers: Modernize tcindex classifier

Jiri Benc (2):
      vxlan: group address requires net device
      tunnels: use macros for IPv6 address comparison

Liping Zhang (1):
      ipmonitor: fix ip monitor can't work when NET_NS is not enabled

Nikolay Aleksandrov (1):
      ip: route: fix multicast route dumps

Or Gerlitz (1):
      devlink: Add e-switch support

Phil Sutter (4):
      man: ip-link.8: Document missing geneve options
      ip-link: add missing {min,max}_tx_rate to help text
      ip-route: Prevent some double spaces in output
      iproute: fix documentation for ip rule scan order

Richard Alpe (2):
      tipc: fix UDP bearer synopsis
      tipc: refactor bearer identification

Roman Mashak (3):
      police: add extra space to improve police result printing
      police: improve usage message
      police: bug fix man page

Roopa Prabhu (2):
      bridge: print_vlan: add missing check for json instance
      bridge: print_vlan: add missing check for json instance

Sabrina Dubroca (4):
      libgenl: introduce genl_init_handle
      macsec: show usage even if the module is not available
      fou: show usage even if the module is not available
      ila: show usage even if the module is not available

Simon Horman (1):
      iproute2: correct port in FOU/GRE example

Stephen Hemminger (14):
      minor header update from net-next
      fib_rules.h update header file
      iprule: whitespace cleanup
      update kernel headers (net-next)
      update kernel header (4.7 net-next)
      update headers files to current net-next
      include: update net-next XDP headers
      update kernel headers
      update BPF headers
      devlink: whitespace cleanup
      remove useless return statement
      ip: iptuntap cleanup
      update kernel headers from 4.8-rc4
      v4.8.0

Sushma Sitaram (1):
      tc: f_u32: Fill in 'linkid' provided by user

Thomas Graf (1):
      tuntap: Add name attribute to usage text

Tom Herbert (6):
      ila: Support for checksum neutral translation
      ila: Support for configuring ila to use netfilter hook
      ip6tnl: Support for fou encapsulation
      gre6: Support for fou encapsulation
      fou: Allowing configuring IPv6 listener
      ipila: Fixed unitialized variables

WANG Cong (1):
      tc: fix a misleading failure

Xin Long (1):
      ip route: restore_handler should check tb[RTA_PREFSRC] for local networks

Yotam Gigi (2):
      tc: Add support for the matchall traffic classifier.
      tc: man: Add man entry for the matchall classifier.

anuradhak (1):
      bridge: Fix garbled json output seen if a vlan filter is specified

^ permalink raw reply

* Re: [PATCH iproute2 6/9] actions:  add skbmod action
From: Stephen Hemminger @ 2016-10-10  2:08 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: netdev
In-Reply-To: <1475354917-7912-7-git-send-email-jhs@emojatatu.com>

On Sat,  1 Oct 2016 16:48:34 -0400
Jamal Hadi Salim <jhs@mojatatu.com> wrote:

> From: Jamal Hadi Salim <jhs@mojatatu.com>
> 
> This action is intended to be an upgrade from a usability perspective
> from pedit (as well as operational debugability).
> Compare this:
> 
> sudo tc filter add dev $ETH parent 1: protocol ip prio 10 \
> u32 match ip protocol 1 0xff flowid 1:2 \
> action pedit munge offset -14 u8 set 0x02 \
>     munge offset -13 u8 set 0x15 \
>     munge offset -12 u8 set 0x15 \
>     munge offset -11 u8 set 0x15 \
>     munge offset -10 u16 set 0x1515 \
>     pipe
> 
> to:
> 
> sudo tc filter add dev $ETH parent 1: protocol ip prio 10 \
> u32 match ip protocol 1 0xff flowid 1:2 \
> action skbmod dmac 02:15:15:15:15:15
> 
> Or worse, try to debug a policy with destination mac, source mac and
> etherype. Then make that a hundred rules and you'll get my point.
> 
> The most important ethernet use case at the moment is when redirecting or
> mirroring packets to a remote machine. The dst mac address needs a re-write
> so that it doesnt get dropped or confuse an interconnecting (learning) switch
> or dropped by a target machine (which looks at the dst mac).
> 
> In the future common use cases on pedit can be migrated to this action
> (as an example different fields in ip v4/6, transports like tcp/udp/sctp
> etc). For this first cut, this allows modifying basic ethernet header.
> 
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

Lots of checkpatch errors on this. Please fix and resubmit series.
For example:

ERROR: spaces required around that '+=' (ctx:WxV)
#442: FILE: tc/m_skbmod.c:79:
+			ok +=1;


ERROR: code indent should use tabs where possible
#567: FILE: tc/m_skbmod.c:204:
+        SPRINT_BUF(b1);$

WARNING: please, no spaces at the start of a line
#567: FILE: tc/m_skbmod.c:204:
+        SPRINT_BUF(b1);$

ERROR: code indent should use tabs where possible
#568: FILE: tc/m_skbmod.c:205:
+        SPRINT_BUF(b2);$

WARNING: please, no spaces at the start of a line
#568: FILE: tc/m_skbmod.c:205:
+        SPRINT_BUF(b2);$

WARNING: braces {} are not necessary for single statement blocks
#610: FILE: tc/m_skbmod.c:247:
+	if (p->flags & SKBMOD_F_SWAPMAC) {
+		fprintf(f, "swap mac ");
+	}


ERROR: trailing whitespace
#816: FILE: man/man8/tc-skbmod.8:28:
+.IR CONTROL " := {"                                                             $

^ permalink raw reply

* Re: [PATCH iproute2-next] tc: fq: display unthrottle latency
From: Stephen Hemminger @ 2016-10-10  2:15 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1475068995.28155.96.camel@edumazet-glaptop3.roam.corp.google.com>

On Wed, 28 Sep 2016 06:23:15 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> From: Eric Dumazet <edumazet@google.com>
> 
> In linux-4.9 fq packet scheduler got a new stat :
> 
> unthrottle_latency in nano second units.
> 
> Gives a good indication of system load or timer implementation
> latencies.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied to net-next (for 4.9)

^ permalink raw reply

* Re: [PATCH iproute2 net-next] ipmroute: add support for age dumping
From: Stephen Hemminger @ 2016-10-10  2:10 UTC (permalink / raw)
  To: Nikolay Aleksandrov; +Cc: netdev, roopa, anuradhak
In-Reply-To: <1474451158-15988-1-git-send-email-nikolay@cumulusnetworks.com>

On Wed, 21 Sep 2016 11:45:58 +0200
Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:

> Add support to dump the mroute cache entry age if the show_stats (-s)
> switch is provided.
> Example:
> $ ip -s mroute
> (0.0.0.0, 239.10.10.10)          Iif: eth0       Oifs: eth0
>   0 packets, 0 bytes, Age  245.44
> 
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

Applied to net-next (pending for 4.9)

^ permalink raw reply

* Re: [PATCH v2 iproute2 net-next] tc: m_vlan: Add vlan modify action
From: Stephen Hemminger @ 2016-10-10  2:12 UTC (permalink / raw)
  To: Shmulik Ladkani; +Cc: netdev, Jamal Hadi Salim, Jiri Pirko
In-Reply-To: <1474567265-3397-1-git-send-email-shmulik.ladkani@gmail.com>

On Thu, 22 Sep 2016 21:01:05 +0300
Shmulik Ladkani <shmulik.ladkani@gmail.com> wrote:

> The 'vlan modify' action allows to replace an existing 802.1q tag
> according to user provided settings.
> It accepts same arguments as the 'vlan push' action.
> 
> For example, this replaces vid 6 with vid 5:
> 
>  # tc filter add dev veth0 parent ffff: pref 1 protocol 802.1q \
>       basic match 'meta(vlan mask 0xfff eq 6)' \
>       action vlan modify id 5 continue
> 
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>

Applied to net-next (for 4.9)

^ permalink raw reply

* Re: [PATCH iproute2] ip link: Add support to configure SR-IOV VF to vlan protocol 802.1ad (VST QinQ)
From: Stephen Hemminger @ 2016-10-10  2:24 UTC (permalink / raw)
  To: Tariq Toukan; +Cc: netdev, Eran Ben Elisha, John Fastabend, Moshe Shemesh
In-Reply-To: <1475049539-25905-1-git-send-email-tariqt@mellanox.com>

On Wed, 28 Sep 2016 10:58:59 +0300
Tariq Toukan <tariqt@mellanox.com> wrote:

> From: Moshe Shemesh <moshe@mellanox.com>
> 
> Introduce a new API that exposes a list of vlans per VF (IFLA_VF_VLAN_LIST),
> giving the ability for user-space application to specify it for the VF as
> an option to support 802.1ad (VST QinQ).
> 
> We introduce struct vf_vlan_info, which extends struct vf_vlan and adds
> an optional VF VLAN proto parameter.
> Default VLAN-protocol is 802.1Q.
> 
> Add IFLA_VF_VLAN_LIST in addition to IFLA_VF_VLAN to keep backward
> compatibility with older kernel versions.
> 
> Suitable ip link tool command examples:
>  - Set vf vlan protocol 802.1ad (S-TAG)
> 	ip link set eth0 vf 1 vlan 100 proto 802.1ad
>  - Set vf vlan S-TAG and vlan C-TAG (VST QinQ)
> 	ip link set eth0 vf 1 vlan 100 proto 802.1ad vlan 30 proto 802.1Q
>  - Set vf to VST (802.1Q) mode
> 	ip link set eth0 vf 1 vlan 100 proto 802.1Q
>  - Or by omitting the new parameter (backward compatible)
> 	ip link set eth0 vf 1 vlan 100
> 
> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>

Applied to net-next (for 4.9)

^ permalink raw reply

* Re: [PATCH iproute2 0/2] ip rule: merger iprule_flush and add selector support
From: Stephen Hemminger @ 2016-10-10  2:30 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev, Phil Sutter
In-Reply-To: <1474525729-2845-1-git-send-email-liuhangbin@gmail.com>

On Thu, 22 Sep 2016 14:28:47 +0800
Hangbin Liu <liuhangbin@gmail.com> wrote:

> When merge iprule_flush() and iprule_list_or_save(). Renamed
> rtnl_filter_t filter to filter_fn because we want to use global
> variable 'filter' to filter nlmsg in the next patch.
> 
> Hangbin Liu (2):
>   ip rule: merge ip rule flush and list, save together
>   ip rule: add selector support
> 
>  ip/iprule.c        | 295 +++++++++++++++++++++++++++++++++++++++++------------
>  man/man8/ip-rule.8 |   6 +-
>  2 files changed, 231 insertions(+), 70 deletions(-)
> 
> -- 

Applied to net-next (for 4.9)

^ permalink raw reply

* Re: net: BUG still has locks held in unix_stream_splice_read
From: Al Viro @ 2016-10-10  2:46 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: David Miller, Hannes Frederic Sowa, Eric Dumazet, netdev, LKML,
	syzkaller
In-Reply-To: <CACT4Y+a6yDm28VYtNPSktYKs_T2UzLGb8jadpFpsdKfNzb7-tA@mail.gmail.com>

On Sun, Oct 09, 2016 at 12:06:14PM +0200, Dmitry Vyukov wrote:
> I suspect this is:
> 
> commit 25869262ef7af24ccde988867ac3eb1c3d4b88d4
> Author: Al Viro <viro@zeniv.linux.org.uk>
> Date:   Sat Sep 17 21:02:10 2016 -0400
>     skb_splice_bits(): get rid of callback
>     since pipe_lock is the outermost now, we don't need to drop/regain
>     socket locks around the call of splice_to_pipe() from skb_splice_bits(),
>     which kills the need to have a socket-specific callback; we can just
>     call splice_to_pipe() and be done with that.

Unlikely, since that particular commit removes unlocking/relocking ->iolock
around the call of splice_to_pipe().  Original would've retaken the same
lock on the way out; it's not as if we could leave the syscall there.

It might be splice-related, but I don't believe that you've got the right
commit here.

^ permalink raw reply

* Re: slab corruption with current -git (was Re: [git pull] vfs pile 1 (splice))
From: Linus Torvalds @ 2016-10-10  2:49 UTC (permalink / raw)
  To: Aaron Conole
  Cc: Florian Westphal, Al Viro, Andrew Morton, Jens Axboe,
	Ted Ts'o, Christoph Lameter, David Miller, Pablo Neira Ayuso,
	Linux Kernel Mailing List, linux-fsdevel, Network Development,
	NetFilter
In-Reply-To: <f7tbmytkmtr.fsf@redhat.com>

On Sun, Oct 9, 2016 at 6:35 PM, Aaron Conole <aconole@redhat.com> wrote:
>
> I was just about to build and test something similar:

So I haven't actually tested that one, but looking at the code, it
really looks very bogus. In fact, that code just looks like crap. It
does *not* do a proper "remove singly linked list entry". It's exactly
the kind of code that I rail against, and that people should never
write.

Any code that can't even traverse a linked list is not worth looking at.

There is one *correct* way to remove an entry from a singly linked
list, and it looks like this:

    struct entry **pp, *p;

    pp = &head;
    while ((p = *pp) != NULL) {
        if (right_entry(p)) {
            *pp = p->next;
            break;
        }
        pp = &p->next;
    }

and that's it. Nothing else. The above code exits the loop with "p"
containing the entry that was removed, or NULL if nothing was. It
can't get any simpler than that, but more importantly, anything more
complicated than that is WRONG.

Seriously, nothing else is acceptable. In particular, any linked list
traversal that makes a special case of the first entry or the last
entry should not be allowed to exist. Note how there is not a single
special case in the above correct code. It JustWorks(tm).

That nf_unregister_net_hook() code has all the signs of exactly that
kind of broken list-handling code: special-casing the head of the
loop, and having the loop condition test both current and that odd
"next to next" pointer etc. It's all very very wrong.

So I really see two options:

 - do that singly-linked list traversal right (and I'm serious:
nothing but the code above can ever be right)

 - don't make up your own list handling code at all, and use the
standard linux list code.

So either e3b37f11e6e4 needs to be reverted, or it needs to be taught
to use real list handling.  If the code doesn't want to use the
regular list.h (either the doubly linked one, or the hlist one), it
needs to at least learn to do list removal right.

               Linus

^ permalink raw reply

* Re: net: BUG still has locks held in unix_stream_splice_read
From: Al Viro @ 2016-10-10  3:14 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: David Miller, Hannes Frederic Sowa, Eric Dumazet, netdev, LKML,
	syzkaller
In-Reply-To: <20161010024607.GV19539@ZenIV.linux.org.uk>

On Mon, Oct 10, 2016 at 03:46:07AM +0100, Al Viro wrote:
> On Sun, Oct 09, 2016 at 12:06:14PM +0200, Dmitry Vyukov wrote:
> > I suspect this is:
> > 
> > commit 25869262ef7af24ccde988867ac3eb1c3d4b88d4
> > Author: Al Viro <viro@zeniv.linux.org.uk>
> > Date:   Sat Sep 17 21:02:10 2016 -0400
> >     skb_splice_bits(): get rid of callback
> >     since pipe_lock is the outermost now, we don't need to drop/regain
> >     socket locks around the call of splice_to_pipe() from skb_splice_bits(),
> >     which kills the need to have a socket-specific callback; we can just
> >     call splice_to_pipe() and be done with that.
> 
> Unlikely, since that particular commit removes unlocking/relocking ->iolock
> around the call of splice_to_pipe().  Original would've retaken the same
> lock on the way out; it's not as if we could leave the syscall there.
> 
> It might be splice-related, but I don't believe that you've got the right
> commit here.

It's not that commit, all right - it's "can't call unix_stream_read_generic()
with any locks held" stepped onto a couple of commits prior by
"splice: lift pipe_lock out of splice_to_pipe()".  Could somebody explain
what is that about?

E.g what will happen if some code does a read on AF_UNIX socket with
some local mutex held?  AFAICS, there are exactly two callers of
freezable_schedule_timeout() - this one and one in XFS; the latter is
in a kernel thread where we do have good warranties about the locking
environment, but here it's in the bleeding ->recvmsg/->splice_read and
for those assumption that caller doesn't hold any locks is pretty
strong, especially since it's not documented anywhere.

What's going on there?

^ permalink raw reply

* [PATCH net] net_sched: do not broadcast RTM_GETTFILTER result
From: Eric Dumazet @ 2016-10-10  3:25 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Jamal Hadi Salim

From: Eric Dumazet <edumazet@google.com>

There are two ways to get tc filters from kernel to user space.

1) Full dump (tc_dump_tfilter())
2) RTM_GETTFILTER to get one precise filter, reducing overhead.

The second operation is unfortunately broadcasting its result,
polluting "tc monitor" users.

This patch makes sure only the requester gets the result, using
netlink_unicast() instead of rtnetlink_send()

Jamal cooked an iproute2 patch to implement "tc filter get" operation,
but other user space libraries already use RTM_GETTFILTER when a single
filter is queried, instead of dumping all filters.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/cls_api.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 11da7da0b7c4..2ee29a3375f6 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -101,7 +101,7 @@ EXPORT_SYMBOL(unregister_tcf_proto_ops);
 
 static int tfilter_notify(struct net *net, struct sk_buff *oskb,
 			  struct nlmsghdr *n, struct tcf_proto *tp,
-			  unsigned long fh, int event);
+			  unsigned long fh, int event, bool unicast);
 
 static void tfilter_notify_chain(struct net *net, struct sk_buff *oskb,
 				 struct nlmsghdr *n,
@@ -112,7 +112,7 @@ static void tfilter_notify_chain(struct net *net, struct sk_buff *oskb,
 
 	for (it_chain = chain; (tp = rtnl_dereference(*it_chain)) != NULL;
 	     it_chain = &tp->next)
-		tfilter_notify(net, oskb, n, tp, 0, event);
+		tfilter_notify(net, oskb, n, tp, 0, event, false);
 }
 
 /* Select new prio value from the range, managed by kernel. */
@@ -319,7 +319,8 @@ replay:
 
 			RCU_INIT_POINTER(*back, next);
 
-			tfilter_notify(net, skb, n, tp, fh, RTM_DELTFILTER);
+			tfilter_notify(net, skb, n, tp, fh,
+				       RTM_DELTFILTER, false);
 			tcf_destroy(tp, true);
 			err = 0;
 			goto errout;
@@ -345,14 +346,14 @@ replay:
 				struct tcf_proto *next = rtnl_dereference(tp->next);
 
 				tfilter_notify(net, skb, n, tp, fh,
-					       RTM_DELTFILTER);
+					       RTM_DELTFILTER, false);
 				if (tcf_destroy(tp, false))
 					RCU_INIT_POINTER(*back, next);
 			}
 			goto errout;
 		case RTM_GETTFILTER:
 			err = tfilter_notify(net, skb, n, tp, fh,
-					     RTM_NEWTFILTER);
+					     RTM_NEWTFILTER, true);
 			goto errout;
 		default:
 			err = -EINVAL;
@@ -367,7 +368,7 @@ replay:
 			RCU_INIT_POINTER(tp->next, rtnl_dereference(*back));
 			rcu_assign_pointer(*back, tp);
 		}
-		tfilter_notify(net, skb, n, tp, fh, RTM_NEWTFILTER);
+		tfilter_notify(net, skb, n, tp, fh, RTM_NEWTFILTER, false);
 	} else {
 		if (tp_created)
 			tcf_destroy(tp, true);
@@ -419,7 +420,7 @@ nla_put_failure:
 
 static int tfilter_notify(struct net *net, struct sk_buff *oskb,
 			  struct nlmsghdr *n, struct tcf_proto *tp,
-			  unsigned long fh, int event)
+			  unsigned long fh, int event, bool unicast)
 {
 	struct sk_buff *skb;
 	u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
@@ -433,6 +434,9 @@ static int tfilter_notify(struct net *net, struct sk_buff *oskb,
 		return -EINVAL;
 	}
 
+	if (unicast)
+		return netlink_unicast(net->rtnl, skb, portid, MSG_DONTWAIT);
+
 	return rtnetlink_send(skb, net, portid, RTNLGRP_TC,
 			      n->nlmsg_flags & NLM_F_ECHO);
 }

^ permalink raw reply related

* Re: slab corruption with current -git (was Re: [git pull] vfs pile 1 (splice))
From: Linus Torvalds @ 2016-10-10  3:41 UTC (permalink / raw)
  To: Aaron Conole
  Cc: Florian Westphal, Al Viro, Andrew Morton, Jens Axboe,
	Ted Ts'o, Christoph Lameter, David Miller, Pablo Neira Ayuso,
	Linux Kernel Mailing List, linux-fsdevel, Network Development,
	NetFilter
In-Reply-To: <CA+55aFzntXuUyL85A68ghS-D-t3VdgKo9-FiqQBG3efW2xns3A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]

On Sun, Oct 9, 2016 at 7:49 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> There is one *correct* way to remove an entry from a singly linked
> list, and it looks like this:
>
>     struct entry **pp, *p;
>
>     pp = &head;
>     while ((p = *pp) != NULL) {
>         if (right_entry(p)) {
>             *pp = p->next;
>             break;
>         }
>         pp = &p->next;
>     }
>
> and that's it. Nothing else.

This COMPLETELY UNTESTED patch tries to fix the nf_hook_entry code to do this.

I repeat: it's ENTIRELY UNTESTED. I just converted the insertion and
deletion to the proper pattern, but I could easily have gotten the
insertion priority test the wrong way around entirely, for example. Or
it could simply have some other completely broken bug in it. It
compiles for me, but that's all I actually checked.

Note that the "correct way" of doing list operations also almost
inevitably is the shortest way by far, since it gets rid of all the
special cases. So the patch looks nice. It gets rid of the magic
"nf_set_hooks_head()" thing too, because once you do list following
right, the head is no different from any other pointer in the list.

So the patch stats look good:

 net/netfilter/core.c | 108 ++++++++++++++++-----------------------------------
 1 file changed, 33 insertions(+), 75 deletions(-)

but again, it's entirely *entirely* untested. Please consider this
just a "this is generally how list insert/delete operations should be
done, avoiding special cases for the first entry".

ALSO NOTE! The code assumes that the "nf_hook_mutex" locking only
protects the actual *lists*, and that the address to the list can be
looked up without holding the lock. That's generally how things are
done, and it simplifies error handling (because you can do the "there
is no such list at all" test before you do anything else. But again, I
don't actually know the code, and if there is something that actually
expands the number of lists etc that depends on that mutex, then the
list head lookup may need to be inside the lock too.

           Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 5008 bytes --]

 net/netfilter/core.c | 108 ++++++++++++++++-----------------------------------
 1 file changed, 33 insertions(+), 75 deletions(-)

diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index c9d90eb64046..814258641fcc 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -65,49 +65,24 @@ static DEFINE_MUTEX(nf_hook_mutex);
 #define nf_entry_dereference(e) \
 	rcu_dereference_protected(e, lockdep_is_held(&nf_hook_mutex))
 
-static struct nf_hook_entry *nf_hook_entry_head(struct net *net,
-						const struct nf_hook_ops *reg)
+static struct nf_hook_entry __rcu **nf_hook_entry_head(struct net *net, const struct nf_hook_ops *reg)
 {
-	struct nf_hook_entry *hook_head = NULL;
-
 	if (reg->pf != NFPROTO_NETDEV)
-		hook_head = nf_entry_dereference(net->nf.hooks[reg->pf]
-						 [reg->hooknum]);
-	else if (reg->hooknum == NF_NETDEV_INGRESS) {
+		return net->nf.hooks[reg->pf]+reg->hooknum;
+
 #ifdef CONFIG_NETFILTER_INGRESS
+	if (reg->hooknum == NF_NETDEV_INGRESS) {
 		if (reg->dev && dev_net(reg->dev) == net)
-			hook_head =
-				nf_entry_dereference(
-					reg->dev->nf_hooks_ingress);
-#endif
+			return &reg->dev->nf_hooks_ingress;
 	}
-	return hook_head;
-}
-
-/* must hold nf_hook_mutex */
-static void nf_set_hooks_head(struct net *net, const struct nf_hook_ops *reg,
-			      struct nf_hook_entry *entry)
-{
-	switch (reg->pf) {
-	case NFPROTO_NETDEV:
-#ifdef CONFIG_NETFILTER_INGRESS
-		/* We already checked in nf_register_net_hook() that this is
-		 * used from ingress.
-		 */
-		rcu_assign_pointer(reg->dev->nf_hooks_ingress, entry);
 #endif
-		break;
-	default:
-		rcu_assign_pointer(net->nf.hooks[reg->pf][reg->hooknum],
-				   entry);
-		break;
-	}
+	return NULL;
 }
 
 int nf_register_net_hook(struct net *net, const struct nf_hook_ops *reg)
 {
-	struct nf_hook_entry *hooks_entry;
-	struct nf_hook_entry *entry;
+	struct nf_hook_entry __rcu **pp;
+	struct nf_hook_entry *entry, *p;
 
 	if (reg->pf == NFPROTO_NETDEV) {
 #ifndef CONFIG_NETFILTER_INGRESS
@@ -119,6 +94,10 @@ int nf_register_net_hook(struct net *net, const struct nf_hook_ops *reg)
 			return -EINVAL;
 	}
 
+	pp = nf_hook_entry_head(net, reg);
+	if (!pp)
+		return -EINVAL;
+
 	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
 	if (!entry)
 		return -ENOMEM;
@@ -128,26 +107,15 @@ int nf_register_net_hook(struct net *net, const struct nf_hook_ops *reg)
 	entry->next	= NULL;
 
 	mutex_lock(&nf_hook_mutex);
-	hooks_entry = nf_hook_entry_head(net, reg);
-
-	if (hooks_entry && hooks_entry->orig_ops->priority > reg->priority) {
-		/* This is the case where we need to insert at the head */
-		entry->next = hooks_entry;
-		hooks_entry = NULL;
-	}
-
-	while (hooks_entry &&
-		reg->priority >= hooks_entry->orig_ops->priority &&
-		nf_entry_dereference(hooks_entry->next)) {
-		hooks_entry = nf_entry_dereference(hooks_entry->next);
-	}
 
-	if (hooks_entry) {
-		entry->next = nf_entry_dereference(hooks_entry->next);
-		rcu_assign_pointer(hooks_entry->next, entry);
-	} else {
-		nf_set_hooks_head(net, reg, entry);
+	/* Find the spot in the list */
+	while ((p = nf_entry_dereference(*pp)) != NULL) {
+		if (reg->priority < p->orig_ops->priority)
+			break;
+		pp = &p->next;
 	}
+	rcu_assign_pointer(entry->next, p);
+	rcu_assign_pointer(*pp, p);
 
 	mutex_unlock(&nf_hook_mutex);
 #ifdef CONFIG_NETFILTER_INGRESS
@@ -163,33 +131,23 @@ EXPORT_SYMBOL(nf_register_net_hook);
 
 void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg)
 {
-	struct nf_hook_entry *hooks_entry;
+	struct nf_hook_entry __rcu **pp;
+	struct nf_hook_entry *p;
 
-	mutex_lock(&nf_hook_mutex);
-	hooks_entry = nf_hook_entry_head(net, reg);
-	if (hooks_entry && hooks_entry->orig_ops == reg) {
-		nf_set_hooks_head(net, reg,
-				  nf_entry_dereference(hooks_entry->next));
-		goto unlock;
-	}
-	while (hooks_entry && nf_entry_dereference(hooks_entry->next)) {
-		struct nf_hook_entry *next =
-			nf_entry_dereference(hooks_entry->next);
-		struct nf_hook_entry *nnext;
+	pp = nf_hook_entry_head(net, reg);
+	if (WARN_ON_ONCE(!pp))
+		return;
 
-		if (next->orig_ops != reg) {
-			hooks_entry = next;
-			continue;
+	mutex_lock(&nf_hook_mutex);
+	while ((p = nf_entry_dereference(*pp)) != NULL) {
+		if (p->orig_ops == reg) {
+			rcu_assign_pointer(*pp, p->next);
+			break;
 		}
-		nnext = nf_entry_dereference(next->next);
-		rcu_assign_pointer(hooks_entry->next, nnext);
-		hooks_entry = next;
-		break;
+		pp = &p->next;
 	}
-
-unlock:
 	mutex_unlock(&nf_hook_mutex);
-	if (!hooks_entry) {
+	if (!p) {
 		WARN(1, "nf_unregister_net_hook: hook not found!\n");
 		return;
 	}
@@ -201,10 +159,10 @@ void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg)
 	static_key_slow_dec(&nf_hooks_needed[reg->pf][reg->hooknum]);
 #endif
 	synchronize_net();
-	nf_queue_nf_hook_drop(net, hooks_entry);
+	nf_queue_nf_hook_drop(net, p);
 	/* other cpu might still process nfqueue verdict that used reg */
 	synchronize_net();
-	kfree(hooks_entry);
+	kfree(p);
 }
 EXPORT_SYMBOL(nf_unregister_net_hook);
 

^ permalink raw reply related

* Re: slab corruption with current -git
From: David Miller @ 2016-10-10  3:57 UTC (permalink / raw)
  To: torvalds
  Cc: aconole, fw, viro, akpm, axboe, tytso, cl, pablo, linux-kernel,
	linux-fsdevel, netdev, netfilter-devel
In-Reply-To: <CA+55aFwS1jfnNzU=8CzRyxC79qKrStwmTXYrBHQYcftAg4LT3Q@mail.gmail.com>

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sun, 9 Oct 2016 20:41:17 -0700

> Note that the "correct way" of doing list operations also almost
> inevitably is the shortest way by far, since it gets rid of all the
> special cases. So the patch looks nice. It gets rid of the magic
> "nf_set_hooks_head()" thing too, because once you do list following
> right, the head is no different from any other pointer in the list.

Perhaps we should have some "slist" primitives added to
include/linux/list.h but since the comparison differs for each user I
guess it's hard to abstract in a way that's generic and inlines
properly.

I'll start taking a look at your patch and this stuff as well, thanks
Linus.

^ permalink raw reply

* Re: [PATCH v2 1/3] net: smc91x: isolate u16 writes alignment workaround
From: Robert Jarzmik @ 2016-10-10  6:30 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rob Herring, Mark Rutland, Nicolas Pitre,
	Russell King - ARM Linux, Arnd Bergmann, netdev, devicetree,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAHp75Vco13D7qQmC8vn6HGg9RYOwR-MgL2uo_cn_OSo4sk84UQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:

>> +#define SMC_outw(lp, v, a, r)                                          \
>> +       _SMC_outw_align4((v), (a), (r),                                 \
>> +                        IS_BUILTIN(CONFIG_ARCH_PXA) && ((r) & 2) &&    \
>> +                        lp->half_word_align4)
>
> Hmm... Isn't enough to have just (r) & 2 && lp->half_word_align4 ?

It wouldn't be equivalent to what we had before.

The point of the previous code was to compile out as much as possible of this
test. Therefore, at compilation time for omap1 boards, the compiler would
evaluate the test to 0, and never leave the workaround code compiled.

So it would be enough, but worse performance wise and not equivalent for non-pxa
boards, hence this test.

Cheers.

-- 
Robert
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 net-next 5/6] qed: Allow chance for fast ramrod completions
From: Mintz, Yuval @ 2016-10-10  6:33 UTC (permalink / raw)
  To: Eric Dumazet, davem@davemloft.net; +Cc: netdev@vger.kernel.org
In-Reply-To: <1476058115.28155.282.camel@edumazet-glaptop3.roam.corp.google.com>


> > +     while (iter_cnt--) {
> > +             /* Validate we receive completion update */
> >                smp_rmb();
> >                if (comp_done->done == 1) {
> >                        if (p_fw_ret)
> >                                *p_fw_ret = comp_done->fw_return_code;
> >                        return 0;
> >                }

> Note that this smp_rmb() and accesses to ->done and ->fw_return_code are
> racy.

> fw_return_code needs to be written _before_ done.

Thanks for catching this up.

Dave  - do you want me to re-spin for this?
I believe it's a day-1 issue [not introduced by this series],
and merits its own patch [and not incorporated into this one].
Still, if you'd like me to re-spin and include Eric's fix, I'll do it.

^ permalink raw reply

* Re: kernel v4.8: iptables logs are truncated with the 4.8 kernel?
From: Chris Caputo @ 2016-10-10  7:02 UTC (permalink / raw)
  To: Vishwanath Pai, Pablo Neira Ayuso; +Cc: Justin Piszcz, linux-kernel, netdev
In-Reply-To: <CAO9zADwLwLLvxmqiLzxK9A5+--ZnvewMzmUvQ4xdm8HHeFNOsA@mail.gmail.com>

On Tue, 4 Oct 2016, Justin Piszcz wrote:
> kernel 4.8 with ulogd-2.0.5- IPs are no longer logged:
> 
> Oct  4 17:51:30 atom INPUT_BLOCK IN=eth1 OUT=
> MAC=00:1b:21:9c:3b:fa:3e:94:d5:d2:49:1e:08:00 LEN=0 TOS=00 PREC=0x00
> TTL=0 ID=0 PROTO=0 MARK=0
> Oct  4 17:51:31 atom INPUT_BLOCK IN=eth1 OUT=
> MAC=00:1b:21:9c:3b:fa:3e:94:d5:d2:49:1e:08:00 LEN=0 TOS=00 PREC=0x00
> TTL=0 ID=0 PROTO=0 MARK=0
> Oct  4 17:51:32 atom INPUT_BLOCK IN=eth1 OUT=
> MAC=00:1b:21:9c:3b:fa:3e:94:d5:d2:49:1e:08:00 LEN=0 TOS=00 PREC=0x00
> TTL=0 ID=0 PROTO=0 MARK=0
> 
> (reboot back to kernel 4.7, works fine)
> 
> kernel 4.7 with ulogd-2.0.5:
> Oct  4 17:56:44 atom INPUT_BLOCK IN=eth1 OUT=
> MAC=00:1b:21:9c:3b:fa:3e:94:d5:d2:49:1e:08:00 SRC=74.125.22.125
> DST=1.2.3.4 LEN=397 TOS=00 PREC=0x00 TTL=48 ID=58093 PROTO=TCP
> SPT=5222 DPT=19804 SEQ=2032644254 ACK=2273184383 WINDOW=55272 ACK PSH
> URGP=0 MARK=0
> Oct  4 17:56:45 atom INPUT_BLOCK IN=eth1 OUT=
> MAC=00:1b:21:9c:3b:fa:3e:94:d5:d2:49:1e:08:00 SRC=74.125.22.125
> DST=1.2.3.4 LEN=397 TOS=00 PREC=0x00 TTL=48 ID=58725 PROTO=TCP
> SPT=5222 DPT=19804 SEQ=2032644254 ACK=2273184383 WINDOW=55272 ACK PSH
> URGP=0 MARK=0
> 
> Looks like there were some changes in the 4.8 kernel regarding ulogd,
> has anyone else run into this problem?

For me, kernel 4.8.1 results in segfaults in ulogd-2.0.5 at:

  Program received signal SIGSEGV, Segmentation fault.
  0x00007ffff65fd18a in _interp_iphdr (pi=0x617f50, len=0) at ulogd_raw2packet_BASE.c:720

  715     static int _interp_iphdr(struct ulogd_pluginstance *pi, uint32_t len)
  716     {
  717             struct ulogd_key *ret = pi->output.keys;
  718             struct iphdr *iph =
  719                     ikey_get_ptr(&pi->input.keys[INKEY_RAW_PCKT]);
  720             void *nexthdr = (uint32_t *)iph + iph->ihl;

I believe 7643507fe8b5bd8ab7522f6a81058cc1209d2585 changed previous 
behavior by not always copying IP header data to user space.

On my machine IPv4 log packets result in a ulogd segfault while IPv6 
packets do not.  I'm not sure of the cause of the difference.

The corresponding userspace commit for the 209d2585 kernel change is:

  https://git.netfilter.org/iptables/commit/?id=7070b1f3c88a0c3d4e315c00cca61f05b0fbc882

This adds --nflog-size to iptables.  When --nflog-size is used with my 
iptables NFLOG lines, the ulogd-2.0.5 segfaults cease.

I'm surprised to see a kernel change cause unexpected userspace segfaults, 
so further investigation into a kernel fix would seem a good idea.  
Having to add the likes of "--nflog-size 200" (200 simply being what I am 
using) to every NFLOG line in firewall configs is a significant burden for 
many.

Putting out a new release of iptables may help ease this transition if the 
kernel is not patched to fix this.  I had to use the git code since 1.6.0 
doesn't have it.

Chris

^ permalink raw reply

* Re: [PATCH net-next 5/6] qed: Allow chance for fast ramrod completions
From: David Miller @ 2016-10-10  7:17 UTC (permalink / raw)
  To: Yuval.Mintz; +Cc: eric.dumazet, netdev
In-Reply-To: <BL2PR07MB230613EB6D587458F4143C678DDB0@BL2PR07MB2306.namprd07.prod.outlook.com>

From: "Mintz, Yuval" <Yuval.Mintz@cavium.com>
Date: Mon, 10 Oct 2016 06:33:05 +0000

> 
>> > +     while (iter_cnt--) {
>> > +             /* Validate we receive completion update */
>> >                smp_rmb();
>> >                if (comp_done->done == 1) {
>> >                        if (p_fw_ret)
>> >                                *p_fw_ret = comp_done->fw_return_code;
>> >                        return 0;
>> >                }
> 
>> Note that this smp_rmb() and accesses to ->done and ->fw_return_code are
>> racy.
> 
>> fw_return_code needs to be written _before_ done.
> 
> Thanks for catching this up.
> 
> Dave  - do you want me to re-spin for this?

Yes.

^ permalink raw reply

* [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning
From: Chris Rorvick @ 2016-10-10  7:19 UTC (permalink / raw)
  To: Intel Linux Wireless, Luca Coelho, Emmanuel Grumbach,
	Johannes Berg, Kalle Valo, Oren Givon
  Cc: linux-wireless, netdev, linux-kernel, Chris Rorvick

Commit bcb079a14d75 ("iwlwifi: pcie: retrieve and parse ACPI power
limitations") looks for a specific structure in the ACPI tables for
setting the default power limit.  The data returned for at least some
dual band chipsets is not recognized, though.  For example, the AC 8260
reports the following:

        Name (SPLX, Package (0x04)
        {
            Zero,
            Package (0x03)
            {
                0,
                1200,
                1000
            },
            Package (0x03)
            {
                0,
                1200,
                1000
            },
            Package (0x03)
            {
                0,
                1200,
                1000
            }
        })

The current logic expects exactly two elements in the outer package,
causing the above to be ignored and the power limit unset.

Despite the interface being fully functional after initialization, the
above condition is reported as an error.  Knock the message down to a
warning and provide better context for understanding its consequence.

Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
 drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
index 78cf9a7..19b531f 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
@@ -540,7 +540,7 @@ static u64 splx_get_pwr_limit(struct iwl_trans *trans, union acpi_object *splx)
 	    splx->package.count != 2 ||
 	    splx->package.elements[0].type != ACPI_TYPE_INTEGER ||
 	    splx->package.elements[0].integer.value != 0) {
-		IWL_ERR(trans, "Unsupported splx structure\n");
+		IWL_WARN(trans, "Unsupported splx structure, not limiting WiFi power\n");
 		return 0;
 	}
 
-- 
2.10.1

^ permalink raw reply related

* Re: [PATCH v2] Add support for ethtool operations to RDC R6040.
From: Florian Fainelli @ 2016-10-10  7:53 UTC (permalink / raw)
  To: VENKAT PRASHANTH B U; +Cc: netdev, fengguang.wu
In-Reply-To: <1475998782-10593-1-git-send-email-venkat.prashanth2498@gmail.com>

On 10/09/2016 12:39 AM, VENKAT PRASHANTH B U wrote:
> Signed-off-by: Venkat Prashanth B U <venkat.prashanth2498@gmail.com>

This should be the last line in your commit message, not the first one.

> 
> Changes since v1:
> 1. Made the commit message more clear
> 2. Add enumeration data type RTL_FLAG_MAX
> 3. Modified the locking interface used in r6040_get_regs()
>    
> 4. Initialized mutex dynamically in a function r6040_get_regs()
>    
> 5. Declared u32 msg_enable in struct r6040_private

The changelog between versions of the patches should be below a --- line
such that it gets ignored when the patch gets applied.

> ---
>  drivers/net/ethernet/rdc/r6040.c | 95 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
> 
> diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
> index cb29ee2..167ff59 100644
> --- a/drivers/net/ethernet/rdc/r6040.c
> +++ b/drivers/net/ethernet/rdc/r6040.c
> @@ -183,6 +183,10 @@ struct r6040_descriptor {
>  	u32	rev2;			/* 1C-1F */
>  } __aligned(32);
>  
> +enum rtl_flag {
> +	RTL_FLAG_MAX
> +};
> +
>  struct r6040_private {
>  	spinlock_t lock;		/* driver lock */
>  	struct pci_dev *pdev;
> @@ -196,12 +200,18 @@ struct r6040_private {
>  	dma_addr_t tx_ring_dma;
>  	u16	tx_free_desc;
>  	u16	mcr0;
> +	u32 msg_enable;
>  	struct net_device *dev;
>  	struct mii_bus *mii_bus;
>  	struct napi_struct napi;
>  	void __iomem *base;
>  	int old_link;
>  	int old_duplex;
> +	struct {
> +		DECLARE_BITMAP(flags, RTL_FLAG_MAX);
> +		struct mutex mutex;
> +		struct work_struct work;
> +	} wk;

Is that necessary?

>  };
>  
>  static char version[] = DRV_NAME
> @@ -955,12 +965,97 @@ static void netdev_get_drvinfo(struct net_device *dev,
>  	strlcpy(info->bus_info, pci_name(rp->pdev), sizeof(info->bus_info));
>  }
>  
> +static void r6040_lock_work(struct r6040_private *tp)
> +{
> +	mutex_lock(&tp->wk.mutex);
> +}
> +
> +static void r6040_unlock_work(struct r6040_private *tp)
> +{
> +	mutex_unlock(&tp->wk.mutex);
> +}
> +
> +static int r6040_get_regs_len (struct net_device *dev)
> +{
> +  return R6040_IO_SIZE;
> +}

Please check your tabs vs. space indentation, kernel coding style is
documented in Documentation/CodingStyle.

> +
> +static void r6040_get_regs (struct net_device *dev, struct ethtool_regs *regs, void *p)
> +{
> +  struct r6040_private *tp = netdev_priv (dev);
> +  u32 __iomem *data = tp->base;
> +  u32 *dw = p;
> +  int i;
> +
> +  r6040_lock_work (tp);
> +  for (i = 0; i < R6040_IO_SIZE; i += 4)
> +    memcpy_fromio (dw++, data++, 4);
> +  r6040_unlock_work (tp);

r6040 registers are typically 16-bits wide, and should be accessed using
ioread16(), did you check this produces the expected result?

> +}
> +
> +static u32 r6040_get_msglevel (struct net_device *dev)
> +{
> +  struct r6040_private *tp = netdev_priv (dev);
> +
> +  return tp->msg_enable;
> +}

That alone does not do anything useful until you start using netif_*
prints in the driver.

> +
> +static void r6040_set_msglevel (struct net_device *dev, u32 value)
> +{
> +  struct r6040_private *tp = netdev_priv (dev);
> +
> +  tp->msg_enable = value;
> +}
> +
> +static const char r6040_gstrings[][ETH_GSTRING_LEN] = {
> +  "tx_packets",
> +  "rx_packets",
> +  "tx_errors",
> +  "rx_errors",
> +  "rx_missed",
> +  "align_errors",
> +  "tx_single_collisions",
> +  "tx_multi_collisions",
> +  "unicast",
> +  "broadcast",
> +  "multicast",
> +  "tx_aborted",
> +  "tx_underrun",
> +};
> +
> +static int r6040_get_sset_count (struct net_device *dev, int sset)
> +{
> +  switch (sset)
> +    {
> +    case ETH_SS_STATS:
> +      return ARRAY_SIZE (r6040_gstrings);
> +    default:
> +      return -EOPNOTSUPP;
> +    }
> +}
> +
> +static void r6040_get_strings (struct net_device *dev, u32 stringset, u8 * data)
> +{
> +  switch (stringset)
> +    {
> +    case ETH_SS_STATS:
> +      memcpy (data, *r6040_gstrings, sizeof (r6040_gstrings));
> +      break;
> +    }
> +}

Where do we actually obtain the statistics from if we do not implement a
get_ethtool_stats callback that fills in these values from either a HW
read or a shadow copy in SW?

Do you have the HW to test these changes?
-- 
Florian

^ permalink raw reply

* Re: net: BUG still has locks held in unix_stream_splice_read
From: Dmitry Vyukov @ 2016-10-10  8:01 UTC (permalink / raw)
  To: Al Viro
  Cc: David Miller, Hannes Frederic Sowa, Eric Dumazet, netdev, LKML,
	syzkaller
In-Reply-To: <20161010031450.GW19539@ZenIV.linux.org.uk>

On Mon, Oct 10, 2016 at 5:14 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Mon, Oct 10, 2016 at 03:46:07AM +0100, Al Viro wrote:
>> On Sun, Oct 09, 2016 at 12:06:14PM +0200, Dmitry Vyukov wrote:
>> > I suspect this is:
>> >
>> > commit 25869262ef7af24ccde988867ac3eb1c3d4b88d4
>> > Author: Al Viro <viro@zeniv.linux.org.uk>
>> > Date:   Sat Sep 17 21:02:10 2016 -0400
>> >     skb_splice_bits(): get rid of callback
>> >     since pipe_lock is the outermost now, we don't need to drop/regain
>> >     socket locks around the call of splice_to_pipe() from skb_splice_bits(),
>> >     which kills the need to have a socket-specific callback; we can just
>> >     call splice_to_pipe() and be done with that.
>>
>> Unlikely, since that particular commit removes unlocking/relocking ->iolock
>> around the call of splice_to_pipe().  Original would've retaken the same
>> lock on the way out; it's not as if we could leave the syscall there.
>>
>> It might be splice-related, but I don't believe that you've got the right
>> commit here.
>
> It's not that commit

It's highly likely. Sorry for falsely pointing to your commit.


> , all right - it's "can't call unix_stream_read_generic()
> with any locks held" stepped onto a couple of commits prior by
> "splice: lift pipe_lock out of splice_to_pipe()".  Could somebody explain
> what is that about?
>
> E.g what will happen if some code does a read on AF_UNIX socket with
> some local mutex held?  AFAICS, there are exactly two callers of
> freezable_schedule_timeout() - this one and one in XFS; the latter is
> in a kernel thread where we do have good warranties about the locking
> environment, but here it's in the bleeding ->recvmsg/->splice_read and
> for those assumption that caller doesn't hold any locks is pretty
> strong, especially since it's not documented anywhere.
>
> What's going on there?

I never saw that warning before. There is some possibility that fuzzer
has discovered some new paths, but it's much more likely that
something has changed recently (the stack looks quite simple -- just a
splice from unix socket). And my previous pull was like a week ago.

^ permalink raw reply

* Re: slab corruption with current -git
From: David Miller @ 2016-10-10  8:24 UTC (permalink / raw)
  To: torvalds
  Cc: aconole, fw, viro, akpm, axboe, tytso, cl, pablo, linux-kernel,
	linux-fsdevel, netdev, netfilter-devel
In-Reply-To: <20161009.235745.860945462339053703.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Sun, 09 Oct 2016 23:57:45 -0400 (EDT)

> From: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Sun, 9 Oct 2016 20:41:17 -0700
> 
>> Note that the "correct way" of doing list operations also almost
>> inevitably is the shortest way by far, since it gets rid of all the
>> special cases. So the patch looks nice. It gets rid of the magic
>> "nf_set_hooks_head()" thing too, because once you do list following
>> right, the head is no different from any other pointer in the list.
> 
> Perhaps we should have some "slist" primitives added to
> include/linux/list.h but since the comparison differs for each user I
> guess it's hard to abstract in a way that's generic and inlines
> properly.
> 
> I'll start taking a look at your patch and this stuff as well, thanks
> Linus.

So I've been reviewing this patch and it looks fine, but I also want
to figure out what is actually causing the OOPS and I can't spot it
yet.

One possible way to see that oops is to free the head entry of the
chain without unlinking it.  The next unregister will dereference a
POISON pointer.

Actually...

The POISON value comes not from a hook entry, but from the array of
pointers in the per-netns datastructure.

This means that the netns is possibly getting freed up before we
unregister the netfilter hooks.

^ permalink raw reply

* 728915 netdev
From: momofr @ 2016-10-10  8:30 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: EMAIL_7265575145931_netdev.zip --]
[-- Type: application/zip, Size: 5471 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox