[GIT] Networking

[GIT] Networking

[David Miller]

1) Handle frames in error situations properly in AF_XDP, from Jakub
   Kicinski.

2) tcp_mmap test case only tests ipv6 due to a thinko, fix from
   Maninder Singh.

3) Session refcnt fix in l2tp_ppp, from Guillaume Nault.

4) Fix regression in netlink bind handling of multicast
   gruops, from Dmitry Safonov.

Please pull, thanks a lot!

The following changes since commit e30cb13c5a09ff5f043a6570c32e49b063bea6a1:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2018-08-02 10:12:02 -0700)

are available in the Git repository at:

  gitolite@ra.kernel.org:/pub/scm/linux/kernel/git/davem/net.git 

for you to fetch changes up to 91874ecf32e41b5d86a4cb9d60e0bee50d828058:

  netlink: Don't shift on 64 for ngroups (2018-08-04 17:52:51 -0700)

----------------------------------------------------------------
Colin Ian King (1):
      drivers: net: lmc: fix case value for target abort error

David S. Miller (2):
      Merge branch 'mlxsw-Fix-ACL-actions-error-condition-handling'
      Merge git://git.kernel.org/.../bpf/bpf

Dmitry Safonov (1):
      netlink: Don't shift on 64 for ngroups

Guillaume Nault (1):
      l2tp: fix missing refcount drop in pppol2tp_tunnel_ioctl()

Jakub Kicinski (1):
      net: xsk: don't return frames via the allocator on error

Maninder Singh (1):
      selftest/net: fix protocol family to work for IPv4.

Mathieu Xhonneux (1):
      selftests/bpf: update test_lwt_seg6local.sh according to iproute2

Nir Dotan (4):
      mlxsw: core_acl_flex_actions: Return error for conflicting actions
      mlxsw: core_acl_flex_actions: Remove redundant resource destruction
      mlxsw: core_acl_flex_actions: Remove redundant counter destruction
      mlxsw: core_acl_flex_actions: Remove redundant mirror resource destruction

Ursula Braun (1):
      net/smc: no cursor update send in state SMC_INIT

Yonghong Song (1):
      tools/bpftool: fix a percpu_array map dump problem

 drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c | 51 +++++++++++++++++++++++++++++----------------------
 drivers/net/wan/lmc/lmc_main.c                              |  2 +-
 net/l2tp/l2tp_ppp.c                                         | 13 +++++++++----
 net/netlink/af_netlink.c                                    |  4 ++--
 net/smc/smc_cdc.c                                           |  3 ++-
 net/xdp/xsk.c                                               |  4 +---
 tools/bpf/bpftool/map.c                                     | 14 +++++++++-----
 tools/testing/selftests/bpf/test_lwt_seg6local.sh           |  6 +++---
 tools/testing/selftests/net/tcp_mmap.c                      |  2 +-
 9 files changed, 57 insertions(+), 42 deletions(-)

Re: [GIT] Networking

[Linus Torvalds]

On Sun, Aug 5, 2018 at 12:47 AM David Miller <davem@davemloft.net> wrote:
>
> 4) Fix regression in netlink bind handling of multicast
>    gruops, from Dmitry Safonov.

This one is written kind of stupidly.

The code went from the original

        groups &= (1UL << nlk->ngroups) - 1;

(which is incorrect for large values of nlk->ngroups) to the fixed

        if (nlk->ngroups == 0)
                groups = 0;
        else if (nlk->ngroups < 8*sizeof(groups))
                groups &= (1UL << nlk->ngroups) - 1;

which isn't technically incorrect...

But it is stupid.

Why stupid? Because the test for 0 is pointless.

Just doing

        if (nlk->ngroups < 8*sizeof(groups))
                groups &= (1UL << nlk->ngroups) - 1;

would have been fine and more understandable, since the "mask by shift
count" already does the right thing for a ngroups value of 0. Now that
test for zero makes me go "what's special about zero?". It turns out
that the answer to that is "nothing".

I certainly didn't care enough to fix it up, and maybe the compiler is
even smart enough to remove the unnecessary test for zero, but it's
kind of sad to see this kind of "people didn't think the code through"
patch this late in the rc.

I'll be making an rc8 today anyway, but I did want to just point to it
and say "hey guys, the code is unnecessarily stupid and overly
complicated".

The type of "groups" is kind of silly too.

Yeah, "long unsigned int" isn't _technically_ wrong. But we normally
call that type "unsigned long".

And comparing against "8*sizeof(groups)" is misleading too, when the
actual masking expression works and is correct in "unsigned long"
because that's the type of the actual mask we're computing (due to the
"1UL").

So _regardless_ of the type of "groups" itself, the mask is actually
correct in unsigned long. I personally think it would have been more
legible as just

        unsigned long groups;
        ...
        if (nlk->ngroups < BITS_PER_LONG)
                groups &= (1UL << nlk->ngroups) - 1;

but by now I'm just nitpicking.

                  Linus

[PATCH] checkpatch: warn on unnecessary int declarations

[Joe Perches]

On Sun, 2018-08-05 at 08:52 -0700, Linus Torvalds wrote:
> "long unsigned int" isn't _technically_ wrong. But we normally
> call that type "unsigned long".

So add a checkpatch test for it.

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ce72cc4784e6..bc6dda34394e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3829,6 +3829,26 @@ sub process {
 			     "type '$tmp' should be specified in [[un]signed] [short|int|long|long long] order\n" . $herecurr);
 		}
 
+# check for unnecessary <signed> int declarations of short/long/long long
+		while ($sline =~ m{\b($TypeMisordered(\s*\*)*|$C90_int_types)\b}g) {
+			my $type = trim($1);
+			next if ($type !~ /\bint\b/);
+			next if ($type !~ /\b(?:short|long\s+long|long)\b/);
+			my $new_type = $type;
+			$new_type =~ s/\b\s*int\s*\b/ /;
+			$new_type =~ s/\b\s*(?:un)?signed\b\s*/ /;
+			$new_type =~ s/^const\s+//;
+			$new_type = "unsigned $new_type" if ($type =~ /\bunsigned\b/);
+			$new_type = "const $new_type" if ($type =~ /^const\b/);
+			$new_type =~ s/\s+/ /g;
+			$new_type = trim($new_type);
+			if (WARN("UNNECESSARY_INT",
+				 "Prefer '$new_type' over '$type' as the int is unnecessary\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$fixlinenr] =~ s/\b\Q$type\E\b/$new_type/;
+			}
+		}
+
 # check for static const char * arrays.
 		if ($line =~ /\bstatic\s+const\s+char\s*\*\s*(\w+)\s*\[\s*\]\s*=\s*/) {
 			WARN("STATIC_CONST_CHAR_ARRAY",

Re: [GIT] Networking

[Dmitry Safonov]

Hi Linus,

2018-08-05 16:52 GMT+01:00 Linus Torvalds <torvalds@linux-foundation.org>:
> On Sun, Aug 5, 2018 at 12:47 AM David Miller <davem@davemloft.net> wrote:
>>
>> 4) Fix regression in netlink bind handling of multicast
>>    gruops, from Dmitry Safonov.
>
> This one is written kind of stupidly.
>
> The code went from the original
>
>         groups &= (1UL << nlk->ngroups) - 1;
>
> (which is incorrect for large values of nlk->ngroups) to the fixed
>
>         if (nlk->ngroups == 0)
>                 groups = 0;
>         else if (nlk->ngroups < 8*sizeof(groups))
>                 groups &= (1UL << nlk->ngroups) - 1;
>
> which isn't technically incorrect...
>
> But it is stupid.
>
> Why stupid? Because the test for 0 is pointless.

Heh, I've been stupid enough at that moment to think that
(1 << 0 == 1) and forgetting that I'm subtracting 1 for mask.

> Just doing
>
>         if (nlk->ngroups < 8*sizeof(groups))
>                 groups &= (1UL << nlk->ngroups) - 1;
>
> would have been fine and more understandable, since the "mask by shift
> count" already does the right thing for a ngroups value of 0. Now that
> test for zero makes me go "what's special about zero?". It turns out
> that the answer to that is "nothing".
>
> I certainly didn't care enough to fix it up, and maybe the compiler is
> even smart enough to remove the unnecessary test for zero, but it's
> kind of sad to see this kind of "people didn't think the code through"
> patch this late in the rc.

Yes, sorry.

> I'll be making an rc8 today anyway, but I did want to just point to it
> and say "hey guys, the code is unnecessarily stupid and overly
> complicated".
>
> The type of "groups" is kind of silly too.
>
> Yeah, "long unsigned int" isn't _technically_ wrong. But we normally
> call that type "unsigned long".
>
> And comparing against "8*sizeof(groups)" is misleading too, when the
> actual masking expression works and is correct in "unsigned long"
> because that's the type of the actual mask we're computing (due to the
> "1UL").
>
> So _regardless_ of the type of "groups" itself, the mask is actually
> correct in unsigned long. I personally think it would have been more
> legible as just
>
>         unsigned long groups;
>         ...
>         if (nlk->ngroups < BITS_PER_LONG)
>                 groups &= (1UL << nlk->ngroups) - 1;
>
> but by now I'm just nitpicking.

I'll prepare the cleanup for linux-next.

Sorry about the stupid code,
             Dmitry