Netdev List
 help / color / mirror / Atom feed
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: David Ahern @ 2018-11-10 14:56 UTC (permalink / raw)
  To: Paweł Staszewski, Jesper Dangaard Brouer; +Cc: netdev, Yoel Caspersen
In-Reply-To: <602b9904-b374-c9a7-abf6-2444d02d1bc0@itcare.pl>

On 11/10/18 6:18 AM, Paweł Staszewski wrote:
> 
> ./xdp_fwd enp175s0f0 enp175s0f1
> libbpf: failed to create map (name: 'stats_map'): Operation not permitted
> libbpf: failed to load object './xdp_fwd_kern.o'

Forgot I had increased locked memory:
 ulimit -l unlimited
./xdp_fwd enp175s0f0 enp175s0f1

^ permalink raw reply

* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Paweł Staszewski @ 2018-11-10 13:18 UTC (permalink / raw)
  To: David Ahern, Jesper Dangaard Brouer; +Cc: netdev, Yoel Caspersen
In-Reply-To: <d3631119-a5fc-b11d-fbad-9a57eeb80f7f@gmail.com>



W dniu 10.11.2018 o 01:06, David Ahern pisze:
> On 11/9/18 9:21 AM, David Ahern wrote:
>>> Is there possible to add only counters from xdp for vlans ?
>>> This will help me in testing.
>> I will take a look today at adding counters that you can dump using
>> bpftool. It will be a temporary solution for this xdp program only.
>>
> Same tree, kernel-tables-wip-02 branch. Compile kernel and install.
> Compile samples as before.
>
> If you give the userspace program a -t arg, it loop showing stats.
> Ctrl-C to break. The xdp programs are not detached on exit.
>
> Example:
>
> ./xdp_fwd -t 5 eth1 eth2 eth3 eth4
>
> 15:59:32:       rx         tx  dropped  skipped    l3_dev    fib_dev
> index  3:   901158     901158        0       18         0          0
> index  4:   901159     901158        0       20         0     901139
> index 10:        0          0        0        0        19         19
> index 11:        0          0        0        0    901139     901139
> index 15:        0          0        0        0        19         19
> index 16:        0          0        0        0    901139          0
>
> Rx and Tx counters are for the physical port.
>
> VLANs show up as l3_dev (ingress) and fib_dev (egress).
>
> dropped is anytime the xdp program returns XDP_DROP (e.g., invalid packet)
>
> skipped is anytime the program returns XDP_PASS (e.g., not ipv4 or ipv6,
> local traffic, or needs full stack assist).
>
recompiled new version but:

./xdp_fwd enp175s0f0 enp175s0f1
libbpf: failed to create map (name: 'stats_map'): Operation not permitted
libbpf: failed to load object './xdp_fwd_kern.o'

^ permalink raw reply

* [PATCH iproute2] testsuite: ss: Fix spacing in expected output for ssfilter.t
From: Stefano Brivio @ 2018-11-10  9:21 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Phil Sutter, Yoann P., netdev

Since commit 00240899ec0b ("ss: Actually print left delimiter for
columns") changes spacing in ss output, we also need to adjust for that in
the ss filter test.

Fixes: 00240899ec0b ("ss: Actually print left delimiter for columns")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 testsuite/tests/ss/ssfilter.t | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/testsuite/tests/ss/ssfilter.t b/testsuite/tests/ss/ssfilter.t
index e74f1765cb72..3091054f2892 100755
--- a/testsuite/tests/ss/ssfilter.t
+++ b/testsuite/tests/ss/ssfilter.t
@@ -12,37 +12,37 @@ export TCPDIAG_FILE="$(dirname $0)/ss1.dump"
 ts_log "[Testing ssfilter]"
 
 ts_ss "$0" "Match dport = 22" -Htna dport = 22
-test_on "ESTAB    0           0                 10.0.0.1:36266           10.0.0.1:22"
+test_on "ESTAB     0           0                10.0.0.1:36266           10.0.0.1:22"
 
 ts_ss "$0" "Match dport 22" -Htna dport 22
-test_on "ESTAB    0           0                 10.0.0.1:36266           10.0.0.1:22"
+test_on "ESTAB     0           0                10.0.0.1:36266           10.0.0.1:22"
 
 ts_ss "$0" "Match (dport)" -Htna '( dport = 22 )'
-test_on "ESTAB    0           0                 10.0.0.1:36266           10.0.0.1:22"
+test_on "ESTAB     0           0                10.0.0.1:36266           10.0.0.1:22"
 
 ts_ss "$0" "Match src = 0.0.0.0" -Htna src = 0.0.0.0
-test_on "LISTEN     0           128                0.0.0.0:22             0.0.0.0:*"
+test_on "LISTEN      0           128               0.0.0.0:22             0.0.0.0:*"
 
 ts_ss "$0" "Match src 0.0.0.0" -Htna src 0.0.0.0
-test_on "LISTEN     0           128                0.0.0.0:22             0.0.0.0:*"
+test_on "LISTEN      0           128               0.0.0.0:22             0.0.0.0:*"
 
 ts_ss "$0" "Match src sport" -Htna src 0.0.0.0 sport = 22
-test_on "LISTEN     0           128                0.0.0.0:22             0.0.0.0:*"
+test_on "LISTEN      0           128               0.0.0.0:22             0.0.0.0:*"
 
 ts_ss "$0" "Match src and sport" -Htna src 0.0.0.0 and sport = 22
-test_on "LISTEN     0           128                0.0.0.0:22             0.0.0.0:*"
+test_on "LISTEN      0           128               0.0.0.0:22             0.0.0.0:*"
 
 ts_ss "$0" "Match src and sport and dport" -Htna src 10.0.0.1 and sport = 22 and dport = 50312
-test_on "ESTAB    0           0                 10.0.0.1:22           10.0.0.2:50312"
+test_on "ESTAB     0           0                10.0.0.1:22           10.0.0.2:50312"
 
 ts_ss "$0" "Match src and sport and (dport)" -Htna 'src 10.0.0.1 and sport = 22 and ( dport = 50312 )'
-test_on "ESTAB    0           0                 10.0.0.1:22           10.0.0.2:50312"
+test_on "ESTAB     0           0                10.0.0.1:22           10.0.0.2:50312"
 
 ts_ss "$0" "Match src and (sport and dport)" -Htna 'src 10.0.0.1 and ( sport = 22 and dport = 50312 )'
-test_on "ESTAB    0           0                 10.0.0.1:22           10.0.0.2:50312"
+test_on "ESTAB     0           0                10.0.0.1:22           10.0.0.2:50312"
 
 ts_ss "$0" "Match (src and sport) and dport" -Htna '( src 10.0.0.1 and sport = 22 ) and dport = 50312'
-test_on "ESTAB    0           0                 10.0.0.1:22           10.0.0.2:50312"
+test_on "ESTAB     0           0                10.0.0.1:22           10.0.0.2:50312"
 
 ts_ss "$0" "Match (src or src) and dst" -Htna '( src 0.0.0.0 or src 10.0.0.1 ) and dst 10.0.0.2'
-test_on "ESTAB    0           0                 10.0.0.1:22           10.0.0.2:50312"
+test_on "ESTAB     0           0                10.0.0.1:22           10.0.0.2:50312"
-- 
2.19.1

^ permalink raw reply related

* Re: [PATCH v4] Wait for running BPF programs when updating map-in-map
From: Chenbo Feng @ 2018-11-10 18:58 UTC (permalink / raw)
  To: gregkh
  Cc: joel, Alexei Starovoitov, Daniel Colascione, Joel Fernandes,
	linux-kernel, Tim Murray, netdev, Lorenzo Colitti,
	Mathieu Desnoyers, Alexei Starovoitov, Daniel Borkmann, stable
In-Reply-To: <20181110152232.GA24724@kroah.com>

The Linus's tree commit SHA is 1ae80cf31938c8f77c37a29bbe29e7f1cd492be8.

I can send the patch to stable directly if needed.

Thanks
Chenbo Feng
On Sat, Nov 10, 2018 at 7:22 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Nov 09, 2018 at 06:01:54PM -0800, Chenbo Feng wrote:
> > Hi netdev,
> >
> > Could we queue up this patch to stable 4.14 and stable 4.19? I can
> > provide a backport patch if needed. I checked it is a clean
> > cherry-pick for 4.19 but have some minor conflict for 4.14.
>
> What is the git commit id of the patch in Linus's tree?
>
> thanks
>
> greg k-h

^ permalink raw reply

* Re: [RFC PATCH 0/3] acpi: Add acpi mdio support code
From: Wang, Dongsheng @ 2018-11-10  9:10 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: timur@kernel.org, Zheng, Joey, f.fainelli@gmail.com,
	rjw@rjwysocki.net, linux-acpi@vger.kernel.org,
	netdev@vger.kernel.org
In-Reply-To: <20181108232353.GL5259@lunn.ch>

Hi Andrew,

On 2018/11/9 7:23, Andrew Lunn wrote:
> On Thu, Nov 08, 2018 at 03:21:29PM +0800, Wang Dongsheng wrote:
>> Originally I just push "phy-handle" support for ACPI on the QCOM QDF2400
>> platform. After some discussion and following Andrew's advice, I send
>> out with a generic version of ACPI.
>>
>> Current there is no clear documentation about MDIO/PHY for ACPI, so when
>> I reading some documents about ACPI [1], I think we just need to reuse the
>> DT binding in the ACPI.[2]. However, this series of patches are not
>> fully compatible with all contents specified in DT binding.
>>
>> The most important thing about this iseries is link the phy device and
>> fwnode of acpi. Besides, we need to carry out bus scan at the mdio
>> register. Therefore, I am not compatible with more DT binding properties
>> in this series of patches. More support will be in the follow-up patches
>> support, or some people do the support.
>>
>> Example:
>> Based on ACPI doc:
>>     Documentation/acpi/dsd/data-node-references.txt
>>     Documentation/acpi/dsd/graph.txt
>> With _DSD device properties we can finally do this:
>>     Device (MDIO) {
>>         Name (_DSD, Package () {
>>             ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>>             Package () { Package () { "ethernet-phy@0", PHY0 }, }
>>         })
>>         Name (PHY0, Package() {
>>             ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>             Package () { Package () { "reg", 0x0 }, }
>>         })
> I don't know much about ACPI. I do know DT. MDIO busses can have
> multiple PHYs on them. Is the following valid to list two PHYs?
>
>      Device (MDIO) {
>          Name (_DSD, Package () {
>              ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>              Package () { Package () { "ethernet-phy@0", PHY0 }, }
>          })
>          Name (PHY0, Package() {
>              ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>              Package () { Package () { "reg", 0x0 }, }
>          })
>          Name (_DSD, Package () {
>              ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>              Package () { Package () { "ethernet-phy@10", PHY1 }, }
>          })
>          Name (PHY1, Package() {
>              ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>              Package () { Package () { "reg", 0x10 }, }
>          })
>      }

Multiple PHYs example:


    Device (MDIO)
    {
        Name (_DSD, Package () {
        ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
            Package () {

                Package () { "ethernet-phy@0", PHY0 },

                Package () { "ethernet-phy@1", PHY1 },
                ...

                ...

            }
        })
       
        Name (PHY0, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "reg", 0x0 },
            }
        })


        Name (PHY1, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "reg", 0x1 },
            }
        })
   }

    Device (MAC0)

    {

        // _DSD: Device-Specific Data
        Name (_DSD, Package (0x02) {
            ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device
Properties for _DSD */,
            Package () {
                Package () { "phy-handle", Package () { \_SB.MDIO,
"ethernet-phy@0" } },
                ...
                ...
            }
        })

    }


    Device (MAC1)

    {

        // _DSD: Device-Specific Data
        Name (_DSD, Package (0x02) {
            ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device
Properties for _DSD */,
            Package () {
                Package () { "phy-handle", Package () { \_SB.MDIO,
"ethernet-phy@1" } },
                ...
                ...
            }
        })

    }

>
> An MDIO bus can also have more than PHYs on them. There can be
> Ethernet switches. Broadcom also have some with generic PHY devices on
> them, and other odd things. That means whatever is on an MDIO bus is a
> device in the Linux device model. How does that work? Do we need some
> form Device (PHY) {}?

ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b") describes a ACPI data node.

The data node can contain property or pointer.


Let's look at the table I'm using:

Device (MAC1)
{
    // _DSD: Device-Specific Data
    Name (_DSD, Package (0x02) {
        ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device
Properties for _DSD */,
        Package (0x07) {
            Package () { "phy-handle", Package () { \_SB.MAC1.MDIO,
"ethernet-phy@1" } },
            Package () { "dev-refs", \_SB.MAC0 },
            Package () { "refs0-dev", Package () { \_SB.MAC1.MDIO,
"refs@0" } },
            Package () { "refs1-dev", Package () { \_SB.MAC1.MDIO,
"refs@1" } },
            ...
            ...
        }
    })

    Device (MDIO)
    {
        Name (_DSD, Package () {
        ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
            Package () {
                Package () { "ethernet-phy@1", PHY1 },
                Package () { "refs@0",  REF0},
                Package () { "refs@1",  REF1},
            }
        })
        
        //Contain a property
        Name (PHY1, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "reg", 0x1 },
            }
        })
        
        //Point to a device
        Name (REF0, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "child-refs-dev", \_SB.MAC0 },
            }
        })
        
        //Contain a property and a pointer that point to a device
        Name (REF1, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "property", 0x5 },
                Package () { "child-refs-dev", \_SB.MAC0 },
            }
        })
    }
}


>      Device (MDIO) {
>          Device (PHY) {
>              Name (_DSD, Package () {
>                  ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>                  Package () { Package () { "ethernet-phy@0", PHY0 }, }
>              })
>              Name (PHY0, Package() {
>                  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                  Package () { Package () { "reg", 0x0 }, }
>              })
>          }
>          Device (PHY) {
>              Name (_DSD, Package () {
>                  ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>                  Package () { Package () { "ethernet-phy@10", PHY1 }, }
>              })
>              Name (PHY1, Package() {
>                  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                  Package () { Package () { "reg", 0x10 }, }
>              })
>          Device (SWITCH) {
>              Name (_DSD, Package () {
>                  ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>                  Package () { Package () { "switch@11", SWITCH0 }, }
>              })
>              Name (SWITCH0, Package() {
>                  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                  Package () { Package () { "reg", 0x11 }, }
>              })
>          }

    Device (MDIO)
    {
        Name (_DSD, Package () {
        ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
            Package () {
                Package () { "ethernet-phy@1", PHY1 },
                Package () { "switch@0",  SW00 },

                Package () { "switch@1",  SW01 },
            }
        })
        
        //Contain a property
        Name (PHY1, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "reg", 0x1 },
            }
        })
 
        Name (SW00, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "reg", 0x0 },
            }
        })

        Name (SW01, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () { "property", 0x5 },
            }
        })
    }


>       }
>
> I'm just trying to ensure whatever is defined is flexible enough that
> we really can later support everything which DT does. We have PHYs on
> MDIO busses, inside switches, which are on MDIO busses, which are
> inside Ethernet interfaces, etc.

I think it can be satisfied. See the table I'm using above.


> An MDIO bus is very similar to an i2c bus. How is that described in
> ACPI? Anything we can learn from that?

About the data node, I just read the Kernel Documentation/acpi/dsd/

And others learn from ACPI spec.


Cheers,

Dongsheng

^ permalink raw reply

* Re: WARNING in format_decode (2)
From: syzbot @ 2018-11-10  8:42 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, linux-kernel, mingo, netdev, rostedt,
	syzkaller-bugs
In-Reply-To: <000000000000e12d4105746dcb0d@google.com>

syzbot has found a reproducer for the following crash on:

HEAD commit:    da85d8bfd151 kselftests/bpf: use ping6 as the default ipv6..
git tree:       bpf
console output: https://syzkaller.appspot.com/x/log.txt?x=116bf533400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=8f559fee2fc3375a
dashboard link: https://syzkaller.appspot.com/bug?extid=1ec5c5ec949c4adaa0c4
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=16d03247400000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=104f5533400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+1ec5c5ec949c4adaa0c4@syzkaller.appspotmail.com

**                                                      **
**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **
**********************************************************
------------[ cut here ]------------
Please remove unsupported %\0 in format string
WARNING: CPU: 0 PID: 5654 at lib/vsprintf.c:2152 format_decode+0x8fc/0xaf0  
lib/vsprintf.c:2152
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 5654 Comm: syz-executor818 Not tainted 4.20.0-rc1+ #80
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x244/0x39d lib/dump_stack.c:113
  panic+0x2ad/0x55c kernel/panic.c:188
  __warn.cold.8+0x20/0x45 kernel/panic.c:540
  report_bug+0x254/0x2d0 lib/bug.c:186
  fixup_bug arch/x86/kernel/traps.c:178 [inline]
  do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
  do_invalid_op+0x36/0x40 arch/x86/kernel/traps.c:290
  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:969
RIP: 0010:format_decode+0x8fc/0xaf0 lib/vsprintf.c:2152
Code: e8 c9 37 b5 f9 41 c6 45 00 12 e9 9b fd ff ff e8 ba 37 b5 f9 0f be f3  
48 c7 c7 c0 f1 d5 88 c6 05 d2 08 47 02 01 e8 74 99 7e f9 <0f> 0b 4d 8b 66  
c0 e9 59 fe ff ff 48 8b bd 70 ff ff ff e8 6d 84 f8
RSP: 0018:ffff8801ba76f670 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff8165e9b5 RDI: 0000000000000005
RBP: ffff8801ba76f718 R08: ffff8801b6cfc0c0 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff8801ba76faec
R13: ffff8801ba76f790 R14: ffff8801ba76f6f0 R15: 0000000000000000
  vsnprintf+0x185/0x1b60 lib/vsprintf.c:2248
  vscnprintf+0x2d/0x80 lib/vsprintf.c:2399
  __trace_array_vprintk.part.59+0x5b/0x2f0 kernel/trace/trace.c:2992
  __trace_array_vprintk kernel/trace/trace.c:3023 [inline]
  trace_array_vprintk kernel/trace/trace.c:3023 [inline]
  trace_vprintk+0x71/0x90 kernel/trace/trace.c:3061
  __trace_printk+0xce/0x120 kernel/trace/trace_printk.c:238
  ____bpf_trace_printk kernel/trace/bpf_trace.c:271 [inline]
  bpf_trace_printk+0xb30/0xc50 kernel/trace/bpf_trace.c:163
  bpf_prog_12183cdb1cd51dab+0x3e7/0x1000
Kernel Offset: disabled
Rebooting in 86400 seconds..


^ permalink raw reply

* Re: [PATCH net v2] net: sched: cls_flower: validate nested enc_opts_policy to avoid warning
From: Jiri Pirko @ 2018-11-10  7:18 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, oss-drivers, netdev
In-Reply-To: <20181110050626.6659-1-jakub.kicinski@netronome.com>

Sat, Nov 10, 2018 at 06:06:26AM CET, jakub.kicinski@netronome.com wrote:
>TCA_FLOWER_KEY_ENC_OPTS and TCA_FLOWER_KEY_ENC_OPTS_MASK can only
>currently contain further nested attributes, which are parsed by
>hand, so the policy is never actually used resulting in a W=1
>build warning:
>
>net/sched/cls_flower.c:492:1: warning: ‘enc_opts_policy’ defined but not used [-Wunused-const-variable=]
> enc_opts_policy[TCA_FLOWER_KEY_ENC_OPTS_MAX + 1] = {
>
>Add the validation anyway to avoid potential bugs when other
>attributes are added and to make the attribute structure slightly
>more clear.  Validation will also set extact to point to bad
>attribute on error.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>Acked-by: Simon Horman <simon.horman@netronome.com>

Fixes: 0a6e77784f49 ("net/sched: allow flower to match tunnel options")
Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [net-next PATCH v3] net: sched: cls_flower: Classify packets using port ranges
From: Jiri Pirko @ 2018-11-10  7:14 UTC (permalink / raw)
  To: Amritha Nambiar
  Cc: netdev, davem, jakub.kicinski, sridhar.samudrala, jhs,
	xiyou.wangcong
In-Reply-To: <154180867008.52041.3062211283036334314.stgit@anamhost.jf.intel.com>

Sat, Nov 10, 2018 at 01:11:10AM CET, amritha.nambiar@intel.com wrote:

[...]

>@@ -1026,8 +1122,7 @@ static void fl_init_dissector(struct flow_dissector *dissector,
> 			     FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4);
> 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> 			     FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6);
>-	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
>-			     FLOW_DISSECTOR_KEY_PORTS, tp);
>+	FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_PORTS, tp);

You still need to set the key under a condition. Something like:
	if (FL_KEY_IS_MASKED(mask, tp) ||
	    FL_KEY_IS_MASKED(mask, tp_min) ||
	    FL_KEY_IS_MASKED(mask, tp_max)
		FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_PORTS, tp);


> 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> 			     FLOW_DISSECTOR_KEY_IP, ip);
> 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,

[...]

^ permalink raw reply

* Re: [PATCH net-next] udp6: cleanup stats accounting in recvmsg()
From: David Miller @ 2018-11-10  4:07 UTC (permalink / raw)
  To: pabeni; +Cc: netdev
In-Reply-To: <650a43db4aa1039a9cefe66aeecf226da9efd115.1541775141.git.pabeni@redhat.com>

From: Paolo Abeni <pabeni@redhat.com>
Date: Fri,  9 Nov 2018 15:52:45 +0100

> In the udp6 code path, we needed multiple tests to select the correct
> mib to be updated. Since we touch at least a counter at each iteration,
> it's convenient to use the recently introduced __UDPX_MIB() helper once
> and remove some code duplication.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next v2 0/2] dpaa2-eth: defer probe on object allocate
From: David Miller @ 2018-11-10  4:09 UTC (permalink / raw)
  To: ioana.ciornei; +Cc: netdev, andrew, ruxandra.radulescu
In-Reply-To: <1541777182-9135-1-git-send-email-ioana.ciornei@nxp.com>

From: Ioana Ciornei <ioana.ciornei@nxp.com>
Date: Fri, 9 Nov 2018 15:26:45 +0000

> Allocatable objects on the fsl-mc bus may be probed by the fsl_mc_allocator
> after the first attempts of other drivers to use them. Defer the probe when
> this situation happens.
> 
> Changes in v2:
>   - proper handling of IS_ERR_OR_NULL

Series applied.

^ permalink raw reply

* Re: [PATCH net] flow_dissector: do not dissect l4 ports for fragments
From: David Miller @ 2018-11-10  4:02 UTC (permalink / raw)
  To: edumazet; +Cc: netdev, eric.dumazet, soukjin.bae
In-Reply-To: <20181110005306.8102-1-edumazet@google.com>

From: Eric Dumazet <edumazet@google.com>
Date: Fri,  9 Nov 2018 16:53:06 -0800

> From: 배석진 <soukjin.bae@samsung.com>
> 
> Only first fragment has the sport/dport information,
> not the following ones.
> 
> If we want consistent hash for all fragments, we need to
> ignore ports even for first fragment.
> 
> This bug is visible for IPv6 traffic, if incoming fragments
> do not have a flow label, since skb_get_hash() will give
> different results for first fragment and following ones.
> 
> It is also visible if any routing rule wants dissection
> and sport or dport.
> 
> See commit 5e5d6fed3741 ("ipv6: route: dissect flow
> in input path if fib rules need it") for details.
> 
> [edumazet] rewrote the changelog completely.
> 
> Fixes: 06635a35d13d ("flow_dissect: use programable dissector in skb_flow_dissect and friends")
> Signed-off-by: 배석진 <soukjin.bae@samsung.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [net-next PATCH v2] net: sched: cls_flower: Classify packets using port ranges
From: Nambiar, Amritha @ 2018-11-10  4:56 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, jakub.kicinski, sridhar.samudrala, jhs,
	xiyou.wangcong
In-Reply-To: <20181109121016.GA4745@nanopsycho>

On 11/9/2018 4:10 AM, Jiri Pirko wrote:
> Wed, Nov 07, 2018 at 10:22:42PM CET, amritha.nambiar@intel.com wrote:
>> Added support in tc flower for filtering based on port ranges.
>>
>> Example:
>> 1. Match on a port range:
>> -------------------------
>> $ tc filter add dev enp4s0 protocol ip parent ffff:\
>>  prio 1 flower ip_proto tcp dst_port range 20-30 skip_hw\
>>  action drop
>>
>> $ tc -s filter show dev enp4s0 parent ffff:
>> filter protocol ip pref 1 flower chain 0
>> filter protocol ip pref 1 flower chain 0 handle 0x1
>>  eth_type ipv4
>>  ip_proto tcp
>>  dst_port range 20-30
>>  skip_hw
>>  not_in_hw
>>        action order 1: gact action drop
>>         random type none pass val 0
>>         index 1 ref 1 bind 1 installed 85 sec used 3 sec
>>        Action statistics:
>>        Sent 460 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)
>>        backlog 0b 0p requeues 0
>>
>> 2. Match on IP address and port range:
>> --------------------------------------
>> $ tc filter add dev enp4s0 protocol ip parent ffff:\
>>  prio 1 flower dst_ip 192.168.1.1 ip_proto tcp dst_port range 100-200\
>>  skip_hw action drop
>>
>> $ tc -s filter show dev enp4s0 parent ffff:
>> filter protocol ip pref 1 flower chain 0 handle 0x2
>>  eth_type ipv4
>>  ip_proto tcp
>>  dst_ip 192.168.1.1
>>  dst_port range 100-200
>>  skip_hw
>>  not_in_hw
>>        action order 1: gact action drop
>>         random type none pass val 0
>>         index 2 ref 1 bind 1 installed 58 sec used 2 sec
>>        Action statistics:
>>        Sent 920 bytes 20 pkt (dropped 20, overlimits 0 requeues 0)
>>        backlog 0b 0p requeues 0
>>
>> v2:
>> Addressed Jiri's comments:
>> 1. Added separate functions for dst and src comparisons.
>> 2. Removed endpoint enum.
>> 3. Added new bit TCA_FLOWER_FLAGS_RANGE to decide normal/range
>>  lookup.
>> 4. Cleaned up fl_lookup function.
>>
>> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
>> ---
>> include/uapi/linux/pkt_cls.h |    7 ++
>> net/sched/cls_flower.c       |  133 ++++++++++++++++++++++++++++++++++++++++--
>> 2 files changed, 134 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
>> index 401d0c1..b63c3cf 100644
>> --- a/include/uapi/linux/pkt_cls.h
>> +++ b/include/uapi/linux/pkt_cls.h
>> @@ -405,6 +405,11 @@ enum {
>> 	TCA_FLOWER_KEY_UDP_SRC,		/* be16 */
>> 	TCA_FLOWER_KEY_UDP_DST,		/* be16 */
>>
>> +	TCA_FLOWER_KEY_PORT_SRC_MIN,	/* be16 */
>> +	TCA_FLOWER_KEY_PORT_SRC_MAX,	/* be16 */
>> +	TCA_FLOWER_KEY_PORT_DST_MIN,	/* be16 */
>> +	TCA_FLOWER_KEY_PORT_DST_MAX,	/* be16 */
>> +
> 
> Please put it at the end of the enum, as David mentioned.

Will fix in v3.

> 
> 
>> 	TCA_FLOWER_FLAGS,
>> 	TCA_FLOWER_KEY_VLAN_ID,		/* be16 */
>> 	TCA_FLOWER_KEY_VLAN_PRIO,	/* u8   */
>> @@ -518,6 +523,8 @@ enum {
>> 	TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
>> };
>>
>> +#define TCA_FLOWER_MASK_FLAGS_RANGE	(1 << 0) /* Range-based match */
>> +
>> /* Match-all classifier */
>>
>> enum {
>> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
>> index 9aada2d..9d2582d 100644
>> --- a/net/sched/cls_flower.c
>> +++ b/net/sched/cls_flower.c
>> @@ -55,6 +55,9 @@ struct fl_flow_key {
>> 	struct flow_dissector_key_ip ip;
>> 	struct flow_dissector_key_ip enc_ip;
>> 	struct flow_dissector_key_enc_opts enc_opts;
>> +
> 
> No need for an empty line.

Will fix in v3.

> 
> 
>> +	struct flow_dissector_key_ports tp_min;
>> +	struct flow_dissector_key_ports tp_max;
>> } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
>>
>> struct fl_flow_mask_range {
>> @@ -65,6 +68,7 @@ struct fl_flow_mask_range {
>> struct fl_flow_mask {
>> 	struct fl_flow_key key;
>> 	struct fl_flow_mask_range range;
>> +	u32 flags;
>> 	struct rhash_head ht_node;
>> 	struct rhashtable ht;
>> 	struct rhashtable_params filter_ht_params;
>> @@ -179,13 +183,89 @@ static void fl_clear_masked_range(struct fl_flow_key *key,
>> 	memset(fl_key_get_start(key, mask), 0, fl_mask_range(mask));
>> }
>>
>> -static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask,
>> -				       struct fl_flow_key *mkey)
>> +static bool fl_range_port_dst_cmp(struct cls_fl_filter *filter,
>> +				  struct fl_flow_key *key,
>> +				  struct fl_flow_key *mkey)
>> +{
>> +	__be16 min_mask, max_mask, min_val, max_val;
>> +
>> +	min_mask = htons(filter->mask->key.tp_min.dst);
>> +	max_mask = htons(filter->mask->key.tp_max.dst);
>> +	min_val = htons(filter->key.tp_min.dst);
>> +	max_val = htons(filter->key.tp_max.dst);
>> +
>> +	if (min_mask && max_mask) {
>> +		if (htons(key->tp.dst) < min_val ||
>> +		    htons(key->tp.dst) > max_val)
>> +			return false;
>> +
>> +		/* skb does not have min and max values */
>> +		mkey->tp_min.dst = filter->mkey.tp_min.dst;
>> +		mkey->tp_max.dst = filter->mkey.tp_max.dst;
>> +	}
>> +	return true;
>> +}
>> +
>> +static bool fl_range_port_src_cmp(struct cls_fl_filter *filter,
>> +				  struct fl_flow_key *key,
>> +				  struct fl_flow_key *mkey)
>> +{
>> +	__be16 min_mask, max_mask, min_val, max_val;
>> +
>> +	min_mask = htons(filter->mask->key.tp_min.src);
>> +	max_mask = htons(filter->mask->key.tp_max.src);
>> +	min_val = htons(filter->key.tp_min.src);
>> +	max_val = htons(filter->key.tp_max.src);
>> +
>> +	if (min_mask && max_mask) {
>> +		if (htons(key->tp.src) < min_val ||
>> +		    htons(key->tp.src) > max_val)
>> +			return false;
>> +
>> +		/* skb does not have min and max values */
>> +		mkey->tp_min.src = filter->mkey.tp_min.src;
>> +		mkey->tp_max.src = filter->mkey.tp_max.src;
>> +	}
>> +	return true;
>> +}
>> +
>> +static struct cls_fl_filter *__fl_lookup(struct fl_flow_mask *mask,
>> +					 struct fl_flow_key *mkey)
>> {
>> 	return rhashtable_lookup_fast(&mask->ht, fl_key_get_start(mkey, mask),
>> 				      mask->filter_ht_params);
>> }
>>
>> +static struct cls_fl_filter *fl_lookup_range(struct fl_flow_mask *mask,
>> +					     struct fl_flow_key *mkey,
>> +					     struct fl_flow_key *key)
>> +{
>> +	struct cls_fl_filter *filter, *f;
>> +
>> +	list_for_each_entry_rcu(filter, &mask->filters, list) {
>> +		if (!fl_range_port_dst_cmp(filter, key, mkey))
>> +			continue;
>> +
>> +		if (!fl_range_port_src_cmp(filter, key, mkey))
>> +			continue;
>> +
>> +		f = __fl_lookup(mask, mkey);
>> +		if (f)
>> +			return f;
>> +	}
>> +	return NULL;
>> +}
>> +
>> +static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask,
>> +				       struct fl_flow_key *mkey,
>> +				       struct fl_flow_key *key)
>> +{
>> +	if ((mask->flags & TCA_FLOWER_MASK_FLAGS_RANGE))
>> +		return fl_lookup_range(mask, mkey, key);
>> +
>> +	return __fl_lookup(mask, mkey);
>> +}
>> +
>> static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
>> 		       struct tcf_result *res)
>> {
>> @@ -207,8 +287,8 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
>> 		skb_flow_dissect(skb, &mask->dissector, &skb_key, 0);
>>
>> 		fl_set_masked_key(&skb_mkey, &skb_key, mask);
>> +		f = fl_lookup(mask, &skb_mkey, &skb_key);
>>
>> -		f = fl_lookup(mask, &skb_mkey);
> 
> Please leave the original ordering (empty line, fl_lookup call).

Will fix in v3.

> 
> 
>> 		if (f && !tc_skip_sw(f->flags)) {
>> 			*res = f->res;
>> 			return tcf_exts_exec(skb, &f->exts, res);
>> @@ -909,6 +989,23 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
>> 			       sizeof(key->arp.tha));
>> 	}
>>
>> +	if (key->basic.ip_proto == IPPROTO_TCP ||
>> +	    key->basic.ip_proto == IPPROTO_UDP ||
>> +	    key->basic.ip_proto == IPPROTO_SCTP) {
>> +		fl_set_key_val(tb, &key->tp_min.dst,
>> +			       TCA_FLOWER_KEY_PORT_DST_MIN, &mask->tp_min.dst,
>> +			       TCA_FLOWER_UNSPEC, sizeof(key->tp_min.dst));
>> +		fl_set_key_val(tb, &key->tp_max.dst,
>> +			       TCA_FLOWER_KEY_PORT_DST_MAX, &mask->tp_max.dst,
>> +			       TCA_FLOWER_UNSPEC, sizeof(key->tp_max.dst));
>> +		fl_set_key_val(tb, &key->tp_min.src,
>> +			       TCA_FLOWER_KEY_PORT_SRC_MIN, &mask->tp_min.src,
>> +			       TCA_FLOWER_UNSPEC, sizeof(key->tp_min.src));
>> +		fl_set_key_val(tb, &key->tp_max.src,
>> +			       TCA_FLOWER_KEY_PORT_SRC_MAX, &mask->tp_max.src,
>> +			       TCA_FLOWER_UNSPEC, sizeof(key->tp_max.src));
>> +	}
>> +
>> 	if (tb[TCA_FLOWER_KEY_ENC_IPV4_SRC] ||
>> 	    tb[TCA_FLOWER_KEY_ENC_IPV4_DST]) {
>> 		key->enc_control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
>> @@ -1026,8 +1123,7 @@ static void fl_init_dissector(struct flow_dissector *dissector,
>> 			     FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4);
>> 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
>> 			     FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6);
>> -	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
>> -			     FLOW_DISSECTOR_KEY_PORTS, tp);
>> +	FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_PORTS, tp);
>> 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
>> 			     FLOW_DISSECTOR_KEY_IP, ip);
>> 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
>> @@ -1074,6 +1170,10 @@ static struct fl_flow_mask *fl_create_new_mask(struct cls_fl_head *head,
>>
>> 	fl_mask_copy(newmask, mask);
>>
>> +	if ((newmask->key.tp_min.dst && newmask->key.tp_max.dst) ||
>> +	    (newmask->key.tp_min.src && newmask->key.tp_max.src))
>> +		newmask->flags |= TCA_FLOWER_MASK_FLAGS_RANGE;
>> +
>> 	err = fl_init_mask_hashtable(newmask);
>> 	if (err)
>> 		goto errout_free;
>> @@ -1227,7 +1327,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
>> 		goto errout_idr;
>>
>> 	if (!tc_skip_sw(fnew->flags)) {
>> -		if (!fold && fl_lookup(fnew->mask, &fnew->mkey)) {
>> +		if (!fold && __fl_lookup(fnew->mask, &fnew->mkey)) {
>> 			err = -EEXIST;
>> 			goto errout_mask;
>> 		}
>> @@ -1800,6 +1900,27 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
>> 				  sizeof(key->arp.tha))))
>> 		goto nla_put_failure;
>>
>> +	if ((key->basic.ip_proto == IPPROTO_TCP ||
>> +	     key->basic.ip_proto == IPPROTO_UDP ||
>> +	     key->basic.ip_proto == IPPROTO_SCTP) &&
>> +	     (fl_dump_key_val(skb, &key->tp_min.dst,
>> +			      TCA_FLOWER_KEY_PORT_DST_MIN,
>> +			      &mask->tp_min.dst, TCA_FLOWER_UNSPEC,
>> +			      sizeof(key->tp_min.dst)) ||
>> +	      fl_dump_key_val(skb, &key->tp_max.dst,
>> +			      TCA_FLOWER_KEY_PORT_DST_MAX,
>> +			      &mask->tp_max.dst, TCA_FLOWER_UNSPEC,
>> +			      sizeof(key->tp_max.dst)) ||
>> +	      fl_dump_key_val(skb, &key->tp_min.src,
>> +			      TCA_FLOWER_KEY_PORT_SRC_MIN,
>> +			      &mask->tp_min.src, TCA_FLOWER_UNSPEC,
>> +			      sizeof(key->tp_min.src)) ||
>> +	      fl_dump_key_val(skb, &key->tp_max.src,
>> +			      TCA_FLOWER_KEY_PORT_SRC_MAX,
>> +			      &mask->tp_max.src, TCA_FLOWER_UNSPEC,
>> +			      sizeof(key->tp_max.src))))
>> +		goto nla_put_failure;
>> +
>> 	if (key->enc_control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
>> 	    (fl_dump_key_val(skb, &key->enc_ipv4.src,
>> 			    TCA_FLOWER_KEY_ENC_IPV4_SRC, &mask->enc_ipv4.src,
>>

^ permalink raw reply

* Re: [net-next PATCH v2] net: sched: cls_flower: Classify packets using port ranges
From: Nambiar, Amritha @ 2018-11-10  4:55 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, jakub.kicinski, sridhar.samudrala, jhs, xiyou.wangcong,
	jiri
In-Reply-To: <20181108.151540.1050553509106510974.davem@davemloft.net>

On 11/8/2018 3:15 PM, David Miller wrote:
> From: Amritha Nambiar <amritha.nambiar@intel.com>
> Date: Wed, 07 Nov 2018 13:22:42 -0800
> 
>> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
>> index 401d0c1..b63c3cf 100644
>> --- a/include/uapi/linux/pkt_cls.h
>> +++ b/include/uapi/linux/pkt_cls.h
>> @@ -405,6 +405,11 @@ enum {
>>  	TCA_FLOWER_KEY_UDP_SRC,		/* be16 */
>>  	TCA_FLOWER_KEY_UDP_DST,		/* be16 */
>>  
>> +	TCA_FLOWER_KEY_PORT_SRC_MIN,	/* be16 */
>> +	TCA_FLOWER_KEY_PORT_SRC_MAX,	/* be16 */
>> +	TCA_FLOWER_KEY_PORT_DST_MIN,	/* be16 */
>> +	TCA_FLOWER_KEY_PORT_DST_MAX,	/* be16 */
>> +
>>  	TCA_FLOWER_FLAGS,
>>  	TCA_FLOWER_KEY_VLAN_ID,		/* be16 */
>>  	TCA_FLOWER_KEY_VLAN_PRIO,	/* u8   */
>> @@ -518,6 +523,8 @@ enum {
> 
> I don't think you can do this without breaking UAPI, this changes the
> value of TCA_FLOWER_FLAGS and all subsequent values in this
> enumeration.
> 

Will move the new fields to the bottom of the enum in v3.

^ permalink raw reply

* Re: [PATCH net] net: sched: cls_flower: validate nested enc_opts_policy to avoid build warning
From: Jakub Kicinski @ 2018-11-10  4:58 UTC (permalink / raw)
  To: David Miller; +Cc: oss-drivers, netdev
In-Reply-To: <20181109.204025.1856075959976079971.davem@davemloft.net>

On Fri, 09 Nov 2018 20:40:25 -0800 (PST), David Miller wrote:
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date: Fri,  9 Nov 2018 14:41:22 -0800
> 
> > TCA_FLOWER_KEY_ENC_OPTS and TCA_FLOWER_KEY_ENC_OPTS_MASK can only
> > currently contain further nested attributes, which are parsed by
> > hand, so the policy is never actually used.  Add the validation
> > anyway to avoid potential bugs when other attributes are added
> > and to make the attribute structure slightly more clear.  Validation
> > will also set extact to point to bad attribute on error.
> > 
> > Fixes: 0a6e77784f49 ("net/sched: allow flower to match tunnel options")
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Acked-by: Simon Horman <simon.horman@netronome.com>  
> 
> If this fixes a build warning, please include the build warning
> message in your commit log.
> 
> Thanks!

Ah, sorry, it's a W=1 warning, which should have been mentioned, too.
I'll repost shortly!

^ permalink raw reply

* Re: [PATCH net] net: sched: cls_flower: validate nested enc_opts_policy to avoid build warning
From: David Miller @ 2018-11-10  4:40 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: oss-drivers, netdev
In-Reply-To: <20181109224122.29609-1-jakub.kicinski@netronome.com>

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri,  9 Nov 2018 14:41:22 -0800

> TCA_FLOWER_KEY_ENC_OPTS and TCA_FLOWER_KEY_ENC_OPTS_MASK can only
> currently contain further nested attributes, which are parsed by
> hand, so the policy is never actually used.  Add the validation
> anyway to avoid potential bugs when other attributes are added
> and to make the attribute structure slightly more clear.  Validation
> will also set extact to point to bad attribute on error.
> 
> Fixes: 0a6e77784f49 ("net/sched: allow flower to match tunnel options")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Acked-by: Simon Horman <simon.horman@netronome.com>

If this fixes a build warning, please include the build warning
message in your commit log.

Thanks!

^ permalink raw reply

* [PATCH net-next 6/6] nfp: flower: remove unnecessary code in flow lookup
From: Jakub Kicinski @ 2018-11-10  5:21 UTC (permalink / raw)
  To: davem
  Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, gerlitz.or, ozsh,
	vladbu, John Hurley
In-Reply-To: <20181110052131.3306-1-jakub.kicinski@netronome.com>

From: John Hurley <john.hurley@netronome.com>

Recent changes to NFP mean that stats updates from fw to driver no longer
require a flow lookup and (because egdev offload has been removed) the
ingress netdev for a lookup is now always known.

Remove obsolete code in a flow lookup that matches on host context and
that allows for a netdev to be NULL.

Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/flower/main.h     |  3 +--
 drivers/net/ethernet/netronome/nfp/flower/metadata.c | 11 +++--------
 drivers/net/ethernet/netronome/nfp/flower/offload.c  |  6 ++----
 3 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.h b/drivers/net/ethernet/netronome/nfp/flower/main.h
index 9d134aa871fc..b858bac47621 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.h
@@ -20,7 +20,6 @@ struct nfp_fl_pre_lag;
 struct net_device;
 struct nfp_app;
 
-#define NFP_FL_STATS_CTX_DONT_CARE	cpu_to_be32(0xffffffff)
 #define NFP_FL_STATS_ELEM_RS		FIELD_SIZEOF(struct nfp_fl_stats_id, \
 						     init_unalloc)
 #define NFP_FLOWER_MASK_ENTRY_RS	256
@@ -242,7 +241,7 @@ int nfp_modify_flow_metadata(struct nfp_app *app,
 
 struct nfp_fl_payload *
 nfp_flower_search_fl_table(struct nfp_app *app, unsigned long tc_flower_cookie,
-			   struct net_device *netdev, __be32 host_ctx);
+			   struct net_device *netdev);
 struct nfp_fl_payload *
 nfp_flower_remove_fl_table(struct nfp_app *app, unsigned long tc_flower_cookie);
 
diff --git a/drivers/net/ethernet/netronome/nfp/flower/metadata.c b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
index 9b4711ce98f0..573a4400a26c 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/metadata.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
@@ -21,7 +21,6 @@ struct nfp_mask_id_table {
 struct nfp_fl_flow_table_cmp_arg {
 	struct net_device *netdev;
 	unsigned long cookie;
-	__be32 host_ctx;
 };
 
 static int nfp_release_stats_entry(struct nfp_app *app, u32 stats_context_id)
@@ -76,14 +75,13 @@ static int nfp_get_stats_entry(struct nfp_app *app, u32 *stats_context_id)
 /* Must be called with either RTNL or rcu_read_lock */
 struct nfp_fl_payload *
 nfp_flower_search_fl_table(struct nfp_app *app, unsigned long tc_flower_cookie,
-			   struct net_device *netdev, __be32 host_ctx)
+			   struct net_device *netdev)
 {
 	struct nfp_fl_flow_table_cmp_arg flower_cmp_arg;
 	struct nfp_flower_priv *priv = app->priv;
 
 	flower_cmp_arg.netdev = netdev;
 	flower_cmp_arg.cookie = tc_flower_cookie;
-	flower_cmp_arg.host_ctx = host_ctx;
 
 	return rhashtable_lookup_fast(&priv->flow_table, &flower_cmp_arg,
 				      nfp_flower_table_params);
@@ -307,8 +305,7 @@ int nfp_compile_flow_metadata(struct nfp_app *app,
 	priv->stats[stats_cxt].bytes = 0;
 	priv->stats[stats_cxt].used = jiffies;
 
-	check_entry = nfp_flower_search_fl_table(app, flow->cookie, netdev,
-						 NFP_FL_STATS_CTX_DONT_CARE);
+	check_entry = nfp_flower_search_fl_table(app, flow->cookie, netdev);
 	if (check_entry) {
 		if (nfp_release_stats_entry(app, stats_cxt))
 			return -EINVAL;
@@ -353,9 +350,7 @@ static int nfp_fl_obj_cmpfn(struct rhashtable_compare_arg *arg,
 	const struct nfp_fl_flow_table_cmp_arg *cmp_arg = arg->key;
 	const struct nfp_fl_payload *flow_entry = obj;
 
-	if ((!cmp_arg->netdev || flow_entry->ingress_dev == cmp_arg->netdev) &&
-	    (cmp_arg->host_ctx == NFP_FL_STATS_CTX_DONT_CARE ||
-	     flow_entry->meta.host_ctx_id == cmp_arg->host_ctx))
+	if (flow_entry->ingress_dev == cmp_arg->netdev)
 		return flow_entry->tc_flower_cookie != cmp_arg->cookie;
 
 	return 1;
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 0e2dfbb3ef86..545d94168874 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -512,8 +512,7 @@ nfp_flower_del_offload(struct nfp_app *app, struct net_device *netdev,
 	if (nfp_netdev_is_nfp_repr(netdev))
 		port = nfp_port_from_netdev(netdev);
 
-	nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, netdev,
-					      NFP_FL_STATS_CTX_DONT_CARE);
+	nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, netdev);
 	if (!nfp_flow)
 		return -ENOENT;
 
@@ -561,8 +560,7 @@ nfp_flower_get_stats(struct nfp_app *app, struct net_device *netdev,
 	struct nfp_fl_payload *nfp_flow;
 	u32 ctx_id;
 
-	nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, netdev,
-					      NFP_FL_STATS_CTX_DONT_CARE);
+	nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, netdev);
 	if (!nfp_flow)
 		return -EINVAL;
 
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v3 1/2] kretprobe: produce sane stack traces
From: Masami Hiramatsu @ 2018-11-10 15:31 UTC (permalink / raw)
  To: Aleksa Sarai
  Cc: Aleksa Sarai, Steven Rostedt, Naveen N. Rao, Anil S Keshavamurthy,
	David S. Miller, Jonathan Corbet, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
	Brendan Gregg, Christian Brauner, netdev, linux-doc, linux-kernel
In-Reply-To: <20181109150629.wpedwxsgbftkl3ab@mikami>

On Sat, 10 Nov 2018 02:06:29 +1100
Aleksa Sarai <asarai@suse.de> wrote:

> On 2018-11-09, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
> > > index ee696efec99f..c4dfafd43e11 100644
> > > --- a/arch/x86/include/asm/ptrace.h
> > > +++ b/arch/x86/include/asm/ptrace.h
> > > @@ -172,6 +172,7 @@ static inline unsigned long kernel_stack_pointer(struct pt_regs *regs)
> > >  	return regs->sp;
> > >  }
> > >  #endif
> > > +#define stack_addr(regs) ((unsigned long *) kernel_stack_pointer(regs))
> > 
> > No, you should use kernel_stack_pointer(regs) itself instead of stack_addr().
> > 
> > > 
> > >  #define GET_IP(regs) ((regs)->ip)
> > >  #define GET_FP(regs) ((regs)->bp)
> > > diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> > > index b0d1e81c96bb..eb4da885020c 100644
> > > --- a/arch/x86/kernel/kprobes/core.c
> > > +++ b/arch/x86/kernel/kprobes/core.c
> > > @@ -69,8 +69,6 @@
> > >  DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
> > >  DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
> > >  
> > > -#define stack_addr(regs) ((unsigned long *)kernel_stack_pointer(regs))
> > 
> > I don't like keeping this meaningless macro... this should be replaced with generic
> > kernel_stack_pointer() macro.
> 
> Sure. This patch was just an example -- I can remove stack_addr() all
> over.
> 
> > > -	if (regs)
> > > -		save_stack_address(trace, regs->ip, nosched);
> > > +	if (regs) {
> > > +		/* XXX: Currently broken -- stack_addr(regs) doesn't match entry. */
> > > +		addr = regs->ip;
> > 
> > Since this part is for storing regs->ip as a top of call-stack, this
> > seems correct code. Stack unwind will be done next block.
> 
> This comment was referring to the usage of stack_addr(). stack_addr()
> doesn't give you the right result (it isn't the address of the return
> address -- it's slightly wrong). This is the main issue I was having --
> am I doing something wrong here?

Of course stack_addr() actually just returns where the stack is. It should
not return address, but maybe a return address from this event happens.
Note that the "regs != NULL" means you will be in the interrupt handler
and it will be returned to the regs->ip.


> > > +		//addr = ftrace_graph_ret_addr(current, &state.graph_idx, addr, stack_addr(regs));
> > 
> > so func graph return trampoline address will be shown only when unwinding stack entries.
> > I mean func-graph tracer is not used as an event, so it never kicks stackdump.
> 
> Just to make sure I understand what you're saying -- func-graph trace
> will never actually call __ftrace_stack_trace? Because if it does, then
> this code will be necessary (and then I'm a bit confused why the
> unwinder has func-graph trace code -- if stack traces are never taken
> under func-graph then the code in the unwinder is not necessary)

You seems misunderstanding. Even if this is not called from func-graph
tracer, the stack entries are already replaced with func-graph trampoline.
However, regs->ip (IRQ return address) is never replaced by the func-graph
trampoline.

> My reason for commenting this out is because at this point "state" isn't
> initialised and thus .graph_idx would not be correctly handled during
> unwind (and it's the same reason I commented it out later).

OK, but anyway, I think we don't need it.

> > > +		addr = kretprobe_ret_addr(current, addr, stack_addr(regs));
> > 
> > But since kretprobe will be an event, which can kick the stackdump.
> > BTW, from kretprobe, regs->ip should always be the trampoline handler, 
> > see arch/x86/kernel/kprobes/core.c:772 :-)
> > So it must be fixed always.
> 
> Right, but kretprobe_ret_addr() is returning the *original* return
> address (and we need to do an (addr == kretprobe_trampoline)). The
> real problem is that stack_addr(regs) isn't the same as it is during
> kretprobe setup (but kretprobe_ret_addr() works everywhere else).

I think stack_addr(regs) should be same when this is called from kretprobe
handler context. Otherwise, yes, it is not same, but in that case, regs->ip
is not kretprobe_trampoline too.

If you find kretprobe_trampoline on the "stack", of course it's address should be
same as it is during kretprobe setup, but if you find kretprobe_trampoline on the
regs->ip, that should always happen on kretprobe handler context. Otherwise,
some critical violation happens on kretprobe_trampoline. In that case, we should
dump the kretprobe_trampoline address itself, should not recover it.

> > > @@ -1856,6 +1870,41 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs)
> > >  }
> > >  NOKPROBE_SYMBOL(pre_handler_kretprobe);
> > >  
> > > +unsigned long kretprobe_ret_addr(struct task_struct *tsk, unsigned long ret,
> > > +				 unsigned long *retp)
> > > +{
> > > +	struct kretprobe_instance *ri;
> > > +	unsigned long flags = 0;
> > > +	struct hlist_head *head;
> > > +	bool need_lock;
> > > +
> > > +	if (likely(ret != (unsigned long) &kretprobe_trampoline))
> > > +		return ret;
> > > +
> > > +	need_lock = !kretprobe_hash_is_locked(tsk);
> > > +	if (WARN_ON(need_lock))
> > > +		kretprobe_hash_lock(tsk, &head, &flags);
> > > +	else
> > > +		head = kretprobe_inst_table_head(tsk);
> > 
> > This may not work unless this is called from the kretprobe handler context,
> > since if we are out of kretprobe handler context, another CPU can lock the
> > hash table and it can be detected by kretprobe_hash_is_locked();.
> 
> Yeah, I noticed this as well when writing it (but needed a quick impl
> that I could test). I will fix this, thanks!
> 
> By is_kretprobe_handler_context() I imagine you are referring to
> checking is_kretprobe(current_kprobe())?

yes, that's correct :)

Thank you,

> 
> > So, we should check we are in the kretprobe handler context if tsk == current,
> > if not, we definately can lock the hash lock without any warning. This can
> > be something like;
> > 
> > if (is_kretprobe_handler_context()) {
> >   // kretprobe_hash_lock(current == tsk) has been locked by caller  
> >   if (tsk != current && kretprobe_hash(tsk) != kretprobe_hash(current))
> >     // the hash of tsk and current can be same.
> >     need_lock = true;
> > } else
> >   // we should take a lock for tsk.
> >   need_lock = true;
> 
> -- 
> Aleksa Sarai
> Senior Software Engineer (Containers)
> SUSE Linux GmbH
> <https://www.cyphar.com/>


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply

* [PATCH net-next 0/6] net: sched: indirect tc block cb registration
From: Jakub Kicinski @ 2018-11-10  5:21 UTC (permalink / raw)
  To: davem
  Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, gerlitz.or, ozsh,
	vladbu, Jakub Kicinski

John says:

This patchset introduces an alternative to egdev offload by allowing a
driver to register for block updates when an external device (e.g. tunnel
netdev) is bound to a TC block. Drivers can track new netdevs or register
to existing ones to receive information on such events. Based on this,
they may register for block offload rules using already existing
functions.

The patchset also implements this new indirect block registration in the
NFP driver to allow the offloading of tunnel rules. The use of egdev
offload (which is currently only used for tunnel offload) is subsequently
removed.

RFC v2 -> PATCH
 - removed embedded tracking function from indir block register (now up to
   driver to clean up after itself)
 - refactored NFP code due to recent submissions
 - removed priv list clean function in NFP (list should be cleared by
   indirect block unregisters)

RFC v1->v2:
 - free allocated owner struct in block_owner_clean function
 - add geneve type helper function
 - move test stub in NFP (v1 patch 2) to full tunnel offload
   implementation via indirect blocks (v2 patches 3-8)

John Hurley (6):
  net: sched: register callbacks for indirect tc block binds
  nfp: flower: allow non repr netdev offload
  nfp: flower: increase scope of netdev checking functions
  nfp: flower: offload tunnel decap rules via indirect TC blocks
  nfp: flower: remove TC egdev offloads
  nfp: flower: remove unnecessary code in flow lookup

 .../ethernet/netronome/nfp/flower/action.c    |  28 +-
 .../net/ethernet/netronome/nfp/flower/cmsg.h  |  27 ++
 .../net/ethernet/netronome/nfp/flower/main.c  |  18 +-
 .../net/ethernet/netronome/nfp/flower/main.h  |  14 +-
 .../net/ethernet/netronome/nfp/flower/match.c |  38 +--
 .../ethernet/netronome/nfp/flower/metadata.c  |  12 +-
 .../ethernet/netronome/nfp/flower/offload.c   | 243 +++++++++++------
 .../netronome/nfp/flower/tunnel_conf.c        |  19 +-
 include/net/pkt_cls.h                         |  34 +++
 include/net/sch_generic.h                     |   3 +
 net/sched/cls_api.c                           | 256 +++++++++++++++++-
 11 files changed, 531 insertions(+), 161 deletions(-)

-- 
2.17.1

^ permalink raw reply

* [PATCH net-next 4/6] nfp: flower: offload tunnel decap rules via indirect TC blocks
From: Jakub Kicinski @ 2018-11-10  5:21 UTC (permalink / raw)
  To: davem
  Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, gerlitz.or, ozsh,
	vladbu, John Hurley
In-Reply-To: <20181110052131.3306-1-jakub.kicinski@netronome.com>

From: John Hurley <john.hurley@netronome.com>

Previously, TC block tunnel decap rules were only offloaded when a
callback was triggered through registration of the rules egress device.
This meant that the driver had no access to the ingress netdev and so
could not verify it was the same tunnel type that the rule implied.

Register tunnel devices for indirect TC block offloads in NFP, giving
access to new rules based on the ingress device rather than egress. Use
this to verify the netdev type of VXLAN and Geneve based rules and offload
the rules to HW if applicable.

Tunnel registration is done via a netdev notifier. On notifier
registration, this is triggered for already existing netdevs. This means
that NFP can register for offloads from devices that exist before it is
loaded (filter rules will be replayed from the TC core). Similarly, on
notifier unregister, a call is triggered for each currently active netdev.
This allows the driver to unregister any indirect block callbacks that may
still be active.

Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 .../net/ethernet/netronome/nfp/flower/main.c  |   6 +
 .../net/ethernet/netronome/nfp/flower/main.h  |   5 +
 .../ethernet/netronome/nfp/flower/offload.c   | 137 +++++++++++++++++-
 3 files changed, 144 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.c b/drivers/net/ethernet/netronome/nfp/flower/main.c
index 2ad00773750f..d1c3c2081461 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.c
@@ -568,6 +568,8 @@ static int nfp_flower_init(struct nfp_app *app)
 		goto err_cleanup_metadata;
 	}
 
+	INIT_LIST_HEAD(&app_priv->indr_block_cb_priv);
+
 	return 0;
 
 err_cleanup_metadata:
@@ -684,6 +686,10 @@ nfp_flower_netdev_event(struct nfp_app *app, struct net_device *netdev,
 			return ret;
 	}
 
+	ret = nfp_flower_reg_indir_block_handler(app, netdev, event);
+	if (ret & NOTIFY_STOP_MASK)
+		return ret;
+
 	return nfp_tunnel_mac_event_handler(app, netdev, event, ptr);
 }
 
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.h b/drivers/net/ethernet/netronome/nfp/flower/main.h
index 4a2b1a915131..8c84829ebd21 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.h
@@ -130,6 +130,7 @@ struct nfp_fl_lag {
  * @reify_wait_queue:	wait queue for repr reify response counting
  * @mtu_conf:		Configuration of repr MTU value
  * @nfp_lag:		Link aggregation data block
+ * @indr_block_cb_priv:	List of priv data passed to indirect block cbs
  */
 struct nfp_flower_priv {
 	struct nfp_app *app;
@@ -162,6 +163,7 @@ struct nfp_flower_priv {
 	wait_queue_head_t reify_wait_queue;
 	struct nfp_mtu_conf mtu_conf;
 	struct nfp_fl_lag nfp_lag;
+	struct list_head indr_block_cb_priv;
 };
 
 /**
@@ -271,5 +273,8 @@ int nfp_flower_lag_populate_pre_action(struct nfp_app *app,
 				       struct nfp_fl_pre_lag *pre_act);
 int nfp_flower_lag_get_output_id(struct nfp_app *app,
 				 struct net_device *master);
+int nfp_flower_reg_indir_block_handler(struct nfp_app *app,
+				       struct net_device *netdev,
+				       unsigned long event);
 
 #endif
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 2c32edfc1a9d..222e1a98cf16 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -128,6 +128,7 @@ nfp_flower_calc_opt_layer(struct flow_dissector_key_enc_opts *enc_opts,
 
 static int
 nfp_flower_calculate_key_layers(struct nfp_app *app,
+				struct net_device *netdev,
 				struct nfp_fl_key_ls *ret_key_ls,
 				struct tc_cls_flower_offload *flow,
 				bool egress,
@@ -186,8 +187,6 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
 			skb_flow_dissector_target(flow->dissector,
 						  FLOW_DISSECTOR_KEY_ENC_CONTROL,
 						  flow->key);
-		if (!egress)
-			return -EOPNOTSUPP;
 
 		if (mask_enc_ctl->addr_type != 0xffff ||
 		    enc_ctl->addr_type != FLOW_DISSECTOR_KEY_IPV4_ADDRS)
@@ -250,6 +249,10 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
 		default:
 			return -EOPNOTSUPP;
 		}
+
+		/* Ensure the ingress netdev matches the expected tun type. */
+		if (!nfp_fl_netdev_is_tunnel_type(netdev, *tun_type))
+			return -EOPNOTSUPP;
 	} else if (egress) {
 		/* Reject non tunnel matches offloaded to egress repr. */
 		return -EOPNOTSUPP;
@@ -451,8 +454,8 @@ nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
 	if (!key_layer)
 		return -ENOMEM;
 
-	err = nfp_flower_calculate_key_layers(app, key_layer, flow, egress,
-					      &tun_type);
+	err = nfp_flower_calculate_key_layers(app, netdev, key_layer, flow,
+					      egress, &tun_type);
 	if (err)
 		goto err_free_key_ls;
 
@@ -693,3 +696,129 @@ int nfp_flower_setup_tc(struct nfp_app *app, struct net_device *netdev,
 		return -EOPNOTSUPP;
 	}
 }
+
+struct nfp_flower_indr_block_cb_priv {
+	struct net_device *netdev;
+	struct nfp_app *app;
+	struct list_head list;
+};
+
+static struct nfp_flower_indr_block_cb_priv *
+nfp_flower_indr_block_cb_priv_lookup(struct nfp_app *app,
+				     struct net_device *netdev)
+{
+	struct nfp_flower_indr_block_cb_priv *cb_priv;
+	struct nfp_flower_priv *priv = app->priv;
+
+	/* All callback list access should be protected by RTNL. */
+	ASSERT_RTNL();
+
+	list_for_each_entry(cb_priv, &priv->indr_block_cb_priv, list)
+		if (cb_priv->netdev == netdev)
+			return cb_priv;
+
+	return NULL;
+}
+
+static int nfp_flower_setup_indr_block_cb(enum tc_setup_type type,
+					  void *type_data, void *cb_priv)
+{
+	struct nfp_flower_indr_block_cb_priv *priv = cb_priv;
+	struct tc_cls_flower_offload *flower = type_data;
+
+	if (flower->common.chain_index)
+		return -EOPNOTSUPP;
+
+	switch (type) {
+	case TC_SETUP_CLSFLOWER:
+		return nfp_flower_repr_offload(priv->app, priv->netdev,
+					       type_data, false);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int
+nfp_flower_setup_indr_tc_block(struct net_device *netdev, struct nfp_app *app,
+			       struct tc_block_offload *f)
+{
+	struct nfp_flower_indr_block_cb_priv *cb_priv;
+	struct nfp_flower_priv *priv = app->priv;
+	int err;
+
+	if (f->binder_type != TCF_BLOCK_BINDER_TYPE_CLSACT_INGRESS)
+		return -EOPNOTSUPP;
+
+	switch (f->command) {
+	case TC_BLOCK_BIND:
+		cb_priv = kmalloc(sizeof(*cb_priv), GFP_KERNEL);
+		if (!cb_priv)
+			return -ENOMEM;
+
+		cb_priv->netdev = netdev;
+		cb_priv->app = app;
+		list_add(&cb_priv->list, &priv->indr_block_cb_priv);
+
+		err = tcf_block_cb_register(f->block,
+					    nfp_flower_setup_indr_block_cb,
+					    netdev, cb_priv, f->extack);
+		if (err) {
+			list_del(&cb_priv->list);
+			kfree(cb_priv);
+		}
+
+		return err;
+	case TC_BLOCK_UNBIND:
+		tcf_block_cb_unregister(f->block,
+					nfp_flower_setup_indr_block_cb, netdev);
+		cb_priv = nfp_flower_indr_block_cb_priv_lookup(app, netdev);
+		if (cb_priv) {
+			list_del(&cb_priv->list);
+			kfree(cb_priv);
+		}
+
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+	return 0;
+}
+
+static int
+nfp_flower_indr_setup_tc_cb(struct net_device *netdev, void *cb_priv,
+			    enum tc_setup_type type, void *type_data)
+{
+	switch (type) {
+	case TC_SETUP_BLOCK:
+		return nfp_flower_setup_indr_tc_block(netdev, cb_priv,
+						      type_data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+int nfp_flower_reg_indir_block_handler(struct nfp_app *app,
+				       struct net_device *netdev,
+				       unsigned long event)
+{
+	int err;
+
+	if (!nfp_fl_is_netdev_to_offload(netdev))
+		return NOTIFY_OK;
+
+	if (event == NETDEV_REGISTER) {
+		err = __tc_indr_block_cb_register(netdev, app,
+						  nfp_flower_indr_setup_tc_cb,
+						  netdev);
+		if (err)
+			nfp_flower_cmsg_warn(app,
+					     "Indirect block reg failed - %s\n",
+					     netdev->name);
+	} else if (event == NETDEV_UNREGISTER) {
+		__tc_indr_block_cb_unregister(netdev,
+					      nfp_flower_indr_setup_tc_cb,
+					      netdev);
+	}
+
+	return NOTIFY_OK;
+}
-- 
2.17.1

^ permalink raw reply related

* [PATCH net-next 5/6] nfp: flower: remove TC egdev offloads
From: Jakub Kicinski @ 2018-11-10  5:21 UTC (permalink / raw)
  To: davem
  Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, gerlitz.or, ozsh,
	vladbu, John Hurley
In-Reply-To: <20181110052131.3306-1-jakub.kicinski@netronome.com>

From: John Hurley <john.hurley@netronome.com>

Previously, only tunnel decap rules required egdev registration for
offload in NFP. These are now supported via indirect TC block callbacks.

Remove the egdev code from NFP.

Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 .../net/ethernet/netronome/nfp/flower/main.c  | 12 ---
 .../net/ethernet/netronome/nfp/flower/main.h  |  3 -
 .../ethernet/netronome/nfp/flower/metadata.c  |  1 +
 .../ethernet/netronome/nfp/flower/offload.c   | 79 ++++---------------
 4 files changed, 17 insertions(+), 78 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.c b/drivers/net/ethernet/netronome/nfp/flower/main.c
index d1c3c2081461..5059110a1768 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.c
@@ -146,23 +146,12 @@ nfp_flower_repr_netdev_stop(struct nfp_app *app, struct nfp_repr *repr)
 	return nfp_flower_cmsg_portmod(repr, false, repr->netdev->mtu, false);
 }
 
-static int
-nfp_flower_repr_netdev_init(struct nfp_app *app, struct net_device *netdev)
-{
-	return tc_setup_cb_egdev_register(netdev,
-					  nfp_flower_setup_tc_egress_cb,
-					  netdev_priv(netdev));
-}
-
 static void
 nfp_flower_repr_netdev_clean(struct nfp_app *app, struct net_device *netdev)
 {
 	struct nfp_repr *repr = netdev_priv(netdev);
 
 	kfree(repr->app_priv);
-
-	tc_setup_cb_egdev_unregister(netdev, nfp_flower_setup_tc_egress_cb,
-				     netdev_priv(netdev));
 }
 
 static void
@@ -711,7 +700,6 @@ const struct nfp_app_type app_flower = {
 	.vnic_init	= nfp_flower_vnic_init,
 	.vnic_clean	= nfp_flower_vnic_clean,
 
-	.repr_init	= nfp_flower_repr_netdev_init,
 	.repr_preclean	= nfp_flower_repr_netdev_preclean,
 	.repr_clean	= nfp_flower_repr_netdev_clean,
 
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.h b/drivers/net/ethernet/netronome/nfp/flower/main.h
index 8c84829ebd21..9d134aa871fc 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.h
@@ -207,7 +207,6 @@ struct nfp_fl_payload {
 	char *unmasked_data;
 	char *mask_data;
 	char *action_data;
-	bool ingress_offload;
 };
 
 extern const struct rhashtable_params nfp_flower_table_params;
@@ -259,8 +258,6 @@ void nfp_tunnel_del_ipv4_off(struct nfp_app *app, __be32 ipv4);
 void nfp_tunnel_add_ipv4_off(struct nfp_app *app, __be32 ipv4);
 void nfp_tunnel_request_route(struct nfp_app *app, struct sk_buff *skb);
 void nfp_tunnel_keep_alive(struct nfp_app *app, struct sk_buff *skb);
-int nfp_flower_setup_tc_egress_cb(enum tc_setup_type type, void *type_data,
-				  void *cb_priv);
 void nfp_flower_lag_init(struct nfp_fl_lag *lag);
 void nfp_flower_lag_cleanup(struct nfp_fl_lag *lag);
 int nfp_flower_lag_reset(struct nfp_fl_lag *lag);
diff --git a/drivers/net/ethernet/netronome/nfp/flower/metadata.c b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
index 48729bf171e0..9b4711ce98f0 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/metadata.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
@@ -287,6 +287,7 @@ int nfp_compile_flow_metadata(struct nfp_app *app,
 
 	nfp_flow->meta.host_ctx_id = cpu_to_be32(stats_cxt);
 	nfp_flow->meta.host_cookie = cpu_to_be64(flow->cookie);
+	nfp_flow->ingress_dev = netdev;
 
 	new_mask_id = 0;
 	if (!nfp_check_mask_add(app, nfp_flow->mask_data,
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 222e1a98cf16..0e2dfbb3ef86 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -131,7 +131,6 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
 				struct net_device *netdev,
 				struct nfp_fl_key_ls *ret_key_ls,
 				struct tc_cls_flower_offload *flow,
-				bool egress,
 				enum nfp_flower_tun_type *tun_type)
 {
 	struct flow_dissector_key_basic *mask_basic = NULL;
@@ -253,9 +252,6 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
 		/* Ensure the ingress netdev matches the expected tun type. */
 		if (!nfp_fl_netdev_is_tunnel_type(netdev, *tun_type))
 			return -EOPNOTSUPP;
-	} else if (egress) {
-		/* Reject non tunnel matches offloaded to egress repr. */
-		return -EOPNOTSUPP;
 	}
 
 	if (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_BASIC)) {
@@ -376,7 +372,7 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
 }
 
 static struct nfp_fl_payload *
-nfp_flower_allocate_new(struct nfp_fl_key_ls *key_layer, bool egress)
+nfp_flower_allocate_new(struct nfp_fl_key_ls *key_layer)
 {
 	struct nfp_fl_payload *flow_pay;
 
@@ -400,7 +396,6 @@ nfp_flower_allocate_new(struct nfp_fl_key_ls *key_layer, bool egress)
 
 	flow_pay->nfp_tun_ipv4_addr = 0;
 	flow_pay->meta.flags = 0;
-	flow_pay->ingress_offload = !egress;
 
 	return flow_pay;
 
@@ -418,7 +413,6 @@ nfp_flower_allocate_new(struct nfp_fl_key_ls *key_layer, bool egress)
  * @app:	Pointer to the APP handle
  * @netdev:	netdev structure.
  * @flow:	TC flower classifier offload structure.
- * @egress:	NFP netdev is the egress.
  *
  * Adds a new flow to the repeated hash structure and action payload.
  *
@@ -426,47 +420,33 @@ nfp_flower_allocate_new(struct nfp_fl_key_ls *key_layer, bool egress)
  */
 static int
 nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
-		       struct tc_cls_flower_offload *flow, bool egress)
+		       struct tc_cls_flower_offload *flow)
 {
 	enum nfp_flower_tun_type tun_type = NFP_FL_TUNNEL_NONE;
 	struct nfp_flower_priv *priv = app->priv;
 	struct nfp_fl_payload *flow_pay;
 	struct nfp_fl_key_ls *key_layer;
 	struct nfp_port *port = NULL;
-	struct net_device *ingr_dev;
 	int err;
 
 	if (nfp_netdev_is_nfp_repr(netdev))
 		port = nfp_port_from_netdev(netdev);
 
-	ingr_dev = egress ? NULL : netdev;
-	flow_pay = nfp_flower_search_fl_table(app, flow->cookie, ingr_dev,
-					      NFP_FL_STATS_CTX_DONT_CARE);
-	if (flow_pay) {
-		/* Ignore as duplicate if it has been added by different cb. */
-		if (flow_pay->ingress_offload && egress)
-			return 0;
-		else
-			return -EOPNOTSUPP;
-	}
-
 	key_layer = kmalloc(sizeof(*key_layer), GFP_KERNEL);
 	if (!key_layer)
 		return -ENOMEM;
 
 	err = nfp_flower_calculate_key_layers(app, netdev, key_layer, flow,
-					      egress, &tun_type);
+					      &tun_type);
 	if (err)
 		goto err_free_key_ls;
 
-	flow_pay = nfp_flower_allocate_new(key_layer, egress);
+	flow_pay = nfp_flower_allocate_new(key_layer);
 	if (!flow_pay) {
 		err = -ENOMEM;
 		goto err_free_key_ls;
 	}
 
-	flow_pay->ingress_dev = egress ? NULL : netdev;
-
 	err = nfp_flower_compile_flow_match(app, flow, key_layer, netdev,
 					    flow_pay, tun_type);
 	if (err)
@@ -476,8 +456,7 @@ nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
 	if (err)
 		goto err_destroy_flow;
 
-	err = nfp_compile_flow_metadata(app, flow, flow_pay,
-					flow_pay->ingress_dev);
+	err = nfp_compile_flow_metadata(app, flow, flow_pay, netdev);
 	if (err)
 		goto err_destroy_flow;
 
@@ -515,7 +494,6 @@ nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
  * @app:	Pointer to the APP handle
  * @netdev:	netdev structure.
  * @flow:	TC flower classifier offload structure
- * @egress:	Netdev is the egress dev.
  *
  * Removes a flow from the repeated hash structure and clears the
  * action payload.
@@ -524,22 +502,20 @@ nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
  */
 static int
 nfp_flower_del_offload(struct nfp_app *app, struct net_device *netdev,
-		       struct tc_cls_flower_offload *flow, bool egress)
+		       struct tc_cls_flower_offload *flow)
 {
 	struct nfp_flower_priv *priv = app->priv;
 	struct nfp_fl_payload *nfp_flow;
 	struct nfp_port *port = NULL;
-	struct net_device *ingr_dev;
 	int err;
 
 	if (nfp_netdev_is_nfp_repr(netdev))
 		port = nfp_port_from_netdev(netdev);
 
-	ingr_dev = egress ? NULL : netdev;
-	nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, ingr_dev,
+	nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, netdev,
 					      NFP_FL_STATS_CTX_DONT_CARE);
 	if (!nfp_flow)
-		return egress ? 0 : -ENOENT;
+		return -ENOENT;
 
 	err = nfp_modify_flow_metadata(app, nfp_flow);
 	if (err)
@@ -571,7 +547,6 @@ nfp_flower_del_offload(struct nfp_app *app, struct net_device *netdev,
  * @app:	Pointer to the APP handle
  * @netdev:	Netdev structure.
  * @flow:	TC flower classifier offload structure
- * @egress:	Netdev is the egress dev.
  *
  * Populates a flow statistics structure which which corresponds to a
  * specific flow.
@@ -580,22 +555,17 @@ nfp_flower_del_offload(struct nfp_app *app, struct net_device *netdev,
  */
 static int
 nfp_flower_get_stats(struct nfp_app *app, struct net_device *netdev,
-		     struct tc_cls_flower_offload *flow, bool egress)
+		     struct tc_cls_flower_offload *flow)
 {
 	struct nfp_flower_priv *priv = app->priv;
 	struct nfp_fl_payload *nfp_flow;
-	struct net_device *ingr_dev;
 	u32 ctx_id;
 
-	ingr_dev = egress ? NULL : netdev;
-	nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, ingr_dev,
+	nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, netdev,
 					      NFP_FL_STATS_CTX_DONT_CARE);
 	if (!nfp_flow)
 		return -EINVAL;
 
-	if (nfp_flow->ingress_offload && egress)
-		return 0;
-
 	ctx_id = be32_to_cpu(nfp_flow->meta.host_ctx_id);
 
 	spin_lock_bh(&priv->stats_lock);
@@ -612,35 +582,18 @@ nfp_flower_get_stats(struct nfp_app *app, struct net_device *netdev,
 
 static int
 nfp_flower_repr_offload(struct nfp_app *app, struct net_device *netdev,
-			struct tc_cls_flower_offload *flower, bool egress)
+			struct tc_cls_flower_offload *flower)
 {
 	if (!eth_proto_is_802_3(flower->common.protocol))
 		return -EOPNOTSUPP;
 
 	switch (flower->command) {
 	case TC_CLSFLOWER_REPLACE:
-		return nfp_flower_add_offload(app, netdev, flower, egress);
+		return nfp_flower_add_offload(app, netdev, flower);
 	case TC_CLSFLOWER_DESTROY:
-		return nfp_flower_del_offload(app, netdev, flower, egress);
+		return nfp_flower_del_offload(app, netdev, flower);
 	case TC_CLSFLOWER_STATS:
-		return nfp_flower_get_stats(app, netdev, flower, egress);
-	default:
-		return -EOPNOTSUPP;
-	}
-}
-
-int nfp_flower_setup_tc_egress_cb(enum tc_setup_type type, void *type_data,
-				  void *cb_priv)
-{
-	struct nfp_repr *repr = cb_priv;
-
-	if (!tc_cls_can_offload_and_chain0(repr->netdev, type_data))
-		return -EOPNOTSUPP;
-
-	switch (type) {
-	case TC_SETUP_CLSFLOWER:
-		return nfp_flower_repr_offload(repr->app, repr->netdev,
-					       type_data, true);
+		return nfp_flower_get_stats(app, netdev, flower);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -657,7 +610,7 @@ static int nfp_flower_setup_tc_block_cb(enum tc_setup_type type,
 	switch (type) {
 	case TC_SETUP_CLSFLOWER:
 		return nfp_flower_repr_offload(repr->app, repr->netdev,
-					       type_data, false);
+					       type_data);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -732,7 +685,7 @@ static int nfp_flower_setup_indr_block_cb(enum tc_setup_type type,
 	switch (type) {
 	case TC_SETUP_CLSFLOWER:
 		return nfp_flower_repr_offload(priv->app, priv->netdev,
-					       type_data, false);
+					       type_data);
 	default:
 		return -EOPNOTSUPP;
 	}
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v4] Wait for running BPF programs when updating map-in-map
From: Greg KH @ 2018-11-10 15:22 UTC (permalink / raw)
  To: Chenbo Feng
  Cc: joel, Alexei Starovoitov, Daniel Colascione, Joel Fernandes,
	linux-kernel, Tim Murray, netdev, Lorenzo Colitti,
	Mathieu Desnoyers, Alexei Starovoitov, Daniel Borkmann, stable
In-Reply-To: <CAMOXUJ=YFEwzBdkQw8xUrQXd4ZRzX6ob0dOCCRA=uEbjvw9yEQ@mail.gmail.com>

On Fri, Nov 09, 2018 at 06:01:54PM -0800, Chenbo Feng wrote:
> Hi netdev,
> 
> Could we queue up this patch to stable 4.14 and stable 4.19? I can
> provide a backport patch if needed. I checked it is a clean
> cherry-pick for 4.19 but have some minor conflict for 4.14.

What is the git commit id of the patch in Linus's tree?

thanks

greg k-h

^ permalink raw reply

* [net-next PATCH v3] net: sched: cls_flower: Classify packets using port ranges
From: Amritha Nambiar @ 2018-11-10  0:11 UTC (permalink / raw)
  To: netdev, davem
  Cc: jakub.kicinski, amritha.nambiar, sridhar.samudrala, jhs,
	xiyou.wangcong, jiri

Added support in tc flower for filtering based on port ranges.

Example:
1. Match on a port range:
-------------------------
$ tc filter add dev enp4s0 protocol ip parent ffff:\
  prio 1 flower ip_proto tcp dst_port range 20-30 skip_hw\
  action drop

$ tc -s filter show dev enp4s0 parent ffff:
filter protocol ip pref 1 flower chain 0
filter protocol ip pref 1 flower chain 0 handle 0x1
  eth_type ipv4
  ip_proto tcp
  dst_port range 20-30
  skip_hw
  not_in_hw
        action order 1: gact action drop
         random type none pass val 0
         index 1 ref 1 bind 1 installed 85 sec used 3 sec
        Action statistics:
        Sent 460 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

2. Match on IP address and port range:
--------------------------------------
$ tc filter add dev enp4s0 protocol ip parent ffff:\
  prio 1 flower dst_ip 192.168.1.1 ip_proto tcp dst_port range 100-200\
  skip_hw action drop

$ tc -s filter show dev enp4s0 parent ffff:
filter protocol ip pref 1 flower chain 0 handle 0x2
  eth_type ipv4
  ip_proto tcp
  dst_ip 192.168.1.1
  dst_port range 100-200
  skip_hw
  not_in_hw
        action order 1: gact action drop
         random type none pass val 0
         index 2 ref 1 bind 1 installed 58 sec used 2 sec
        Action statistics:
        Sent 920 bytes 20 pkt (dropped 20, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

v3:
1. Moved new fields in UAPI enum to the end of enum.
2. Removed couple of empty lines.

v2:
Addressed Jiri's comments:
1. Added separate functions for dst and src comparisons.
2. Removed endpoint enum.
3. Added new bit TCA_FLOWER_FLAGS_RANGE to decide normal/range
  lookup.
4. Cleaned up fl_lookup function.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
---
 include/uapi/linux/pkt_cls.h |    7 ++
 net/sched/cls_flower.c       |  132 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 133 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 401d0c1..95d0db2 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -485,6 +485,11 @@ enum {
 
 	TCA_FLOWER_IN_HW_COUNT,
 
+	TCA_FLOWER_KEY_PORT_SRC_MIN,	/* be16 */
+	TCA_FLOWER_KEY_PORT_SRC_MAX,	/* be16 */
+	TCA_FLOWER_KEY_PORT_DST_MIN,	/* be16 */
+	TCA_FLOWER_KEY_PORT_DST_MAX,	/* be16 */
+
 	__TCA_FLOWER_MAX,
 };
 
@@ -518,6 +523,8 @@ enum {
 	TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
 };
 
+#define TCA_FLOWER_MASK_FLAGS_RANGE	(1 << 0) /* Range-based match */
+
 /* Match-all classifier */
 
 enum {
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 9aada2d..7780106 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -55,6 +55,8 @@ struct fl_flow_key {
 	struct flow_dissector_key_ip ip;
 	struct flow_dissector_key_ip enc_ip;
 	struct flow_dissector_key_enc_opts enc_opts;
+	struct flow_dissector_key_ports tp_min;
+	struct flow_dissector_key_ports tp_max;
 } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
 
 struct fl_flow_mask_range {
@@ -65,6 +67,7 @@ struct fl_flow_mask_range {
 struct fl_flow_mask {
 	struct fl_flow_key key;
 	struct fl_flow_mask_range range;
+	u32 flags;
 	struct rhash_head ht_node;
 	struct rhashtable ht;
 	struct rhashtable_params filter_ht_params;
@@ -179,13 +182,89 @@ static void fl_clear_masked_range(struct fl_flow_key *key,
 	memset(fl_key_get_start(key, mask), 0, fl_mask_range(mask));
 }
 
-static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask,
-				       struct fl_flow_key *mkey)
+static bool fl_range_port_dst_cmp(struct cls_fl_filter *filter,
+				  struct fl_flow_key *key,
+				  struct fl_flow_key *mkey)
+{
+	__be16 min_mask, max_mask, min_val, max_val;
+
+	min_mask = htons(filter->mask->key.tp_min.dst);
+	max_mask = htons(filter->mask->key.tp_max.dst);
+	min_val = htons(filter->key.tp_min.dst);
+	max_val = htons(filter->key.tp_max.dst);
+
+	if (min_mask && max_mask) {
+		if (htons(key->tp.dst) < min_val ||
+		    htons(key->tp.dst) > max_val)
+			return false;
+
+		/* skb does not have min and max values */
+		mkey->tp_min.dst = filter->mkey.tp_min.dst;
+		mkey->tp_max.dst = filter->mkey.tp_max.dst;
+	}
+	return true;
+}
+
+static bool fl_range_port_src_cmp(struct cls_fl_filter *filter,
+				  struct fl_flow_key *key,
+				  struct fl_flow_key *mkey)
+{
+	__be16 min_mask, max_mask, min_val, max_val;
+
+	min_mask = htons(filter->mask->key.tp_min.src);
+	max_mask = htons(filter->mask->key.tp_max.src);
+	min_val = htons(filter->key.tp_min.src);
+	max_val = htons(filter->key.tp_max.src);
+
+	if (min_mask && max_mask) {
+		if (htons(key->tp.src) < min_val ||
+		    htons(key->tp.src) > max_val)
+			return false;
+
+		/* skb does not have min and max values */
+		mkey->tp_min.src = filter->mkey.tp_min.src;
+		mkey->tp_max.src = filter->mkey.tp_max.src;
+	}
+	return true;
+}
+
+static struct cls_fl_filter *__fl_lookup(struct fl_flow_mask *mask,
+					 struct fl_flow_key *mkey)
 {
 	return rhashtable_lookup_fast(&mask->ht, fl_key_get_start(mkey, mask),
 				      mask->filter_ht_params);
 }
 
+static struct cls_fl_filter *fl_lookup_range(struct fl_flow_mask *mask,
+					     struct fl_flow_key *mkey,
+					     struct fl_flow_key *key)
+{
+	struct cls_fl_filter *filter, *f;
+
+	list_for_each_entry_rcu(filter, &mask->filters, list) {
+		if (!fl_range_port_dst_cmp(filter, key, mkey))
+			continue;
+
+		if (!fl_range_port_src_cmp(filter, key, mkey))
+			continue;
+
+		f = __fl_lookup(mask, mkey);
+		if (f)
+			return f;
+	}
+	return NULL;
+}
+
+static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask,
+				       struct fl_flow_key *mkey,
+				       struct fl_flow_key *key)
+{
+	if ((mask->flags & TCA_FLOWER_MASK_FLAGS_RANGE))
+		return fl_lookup_range(mask, mkey, key);
+
+	return __fl_lookup(mask, mkey);
+}
+
 static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 		       struct tcf_result *res)
 {
@@ -208,7 +287,7 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 
 		fl_set_masked_key(&skb_mkey, &skb_key, mask);
 
-		f = fl_lookup(mask, &skb_mkey);
+		f = fl_lookup(mask, &skb_mkey, &skb_key);
 		if (f && !tc_skip_sw(f->flags)) {
 			*res = f->res;
 			return tcf_exts_exec(skb, &f->exts, res);
@@ -909,6 +988,23 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
 			       sizeof(key->arp.tha));
 	}
 
+	if (key->basic.ip_proto == IPPROTO_TCP ||
+	    key->basic.ip_proto == IPPROTO_UDP ||
+	    key->basic.ip_proto == IPPROTO_SCTP) {
+		fl_set_key_val(tb, &key->tp_min.dst,
+			       TCA_FLOWER_KEY_PORT_DST_MIN, &mask->tp_min.dst,
+			       TCA_FLOWER_UNSPEC, sizeof(key->tp_min.dst));
+		fl_set_key_val(tb, &key->tp_max.dst,
+			       TCA_FLOWER_KEY_PORT_DST_MAX, &mask->tp_max.dst,
+			       TCA_FLOWER_UNSPEC, sizeof(key->tp_max.dst));
+		fl_set_key_val(tb, &key->tp_min.src,
+			       TCA_FLOWER_KEY_PORT_SRC_MIN, &mask->tp_min.src,
+			       TCA_FLOWER_UNSPEC, sizeof(key->tp_min.src));
+		fl_set_key_val(tb, &key->tp_max.src,
+			       TCA_FLOWER_KEY_PORT_SRC_MAX, &mask->tp_max.src,
+			       TCA_FLOWER_UNSPEC, sizeof(key->tp_max.src));
+	}
+
 	if (tb[TCA_FLOWER_KEY_ENC_IPV4_SRC] ||
 	    tb[TCA_FLOWER_KEY_ENC_IPV4_DST]) {
 		key->enc_control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
@@ -1026,8 +1122,7 @@ static void fl_init_dissector(struct flow_dissector *dissector,
 			     FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4);
 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6);
-	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
-			     FLOW_DISSECTOR_KEY_PORTS, tp);
+	FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_PORTS, tp);
 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_IP, ip);
 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
@@ -1074,6 +1169,10 @@ static struct fl_flow_mask *fl_create_new_mask(struct cls_fl_head *head,
 
 	fl_mask_copy(newmask, mask);
 
+	if ((newmask->key.tp_min.dst && newmask->key.tp_max.dst) ||
+	    (newmask->key.tp_min.src && newmask->key.tp_max.src))
+		newmask->flags |= TCA_FLOWER_MASK_FLAGS_RANGE;
+
 	err = fl_init_mask_hashtable(newmask);
 	if (err)
 		goto errout_free;
@@ -1227,7 +1326,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
 		goto errout_idr;
 
 	if (!tc_skip_sw(fnew->flags)) {
-		if (!fold && fl_lookup(fnew->mask, &fnew->mkey)) {
+		if (!fold && __fl_lookup(fnew->mask, &fnew->mkey)) {
 			err = -EEXIST;
 			goto errout_mask;
 		}
@@ -1800,6 +1899,27 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
 				  sizeof(key->arp.tha))))
 		goto nla_put_failure;
 
+	if ((key->basic.ip_proto == IPPROTO_TCP ||
+	     key->basic.ip_proto == IPPROTO_UDP ||
+	     key->basic.ip_proto == IPPROTO_SCTP) &&
+	     (fl_dump_key_val(skb, &key->tp_min.dst,
+			      TCA_FLOWER_KEY_PORT_DST_MIN,
+			      &mask->tp_min.dst, TCA_FLOWER_UNSPEC,
+			      sizeof(key->tp_min.dst)) ||
+	      fl_dump_key_val(skb, &key->tp_max.dst,
+			      TCA_FLOWER_KEY_PORT_DST_MAX,
+			      &mask->tp_max.dst, TCA_FLOWER_UNSPEC,
+			      sizeof(key->tp_max.dst)) ||
+	      fl_dump_key_val(skb, &key->tp_min.src,
+			      TCA_FLOWER_KEY_PORT_SRC_MIN,
+			      &mask->tp_min.src, TCA_FLOWER_UNSPEC,
+			      sizeof(key->tp_min.src)) ||
+	      fl_dump_key_val(skb, &key->tp_max.src,
+			      TCA_FLOWER_KEY_PORT_SRC_MAX,
+			      &mask->tp_max.src, TCA_FLOWER_UNSPEC,
+			      sizeof(key->tp_max.src))))
+		goto nla_put_failure;
+
 	if (key->enc_control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
 	    (fl_dump_key_val(skb, &key->enc_ipv4.src,
 			    TCA_FLOWER_KEY_ENC_IPV4_SRC, &mask->enc_ipv4.src,

^ permalink raw reply related

* Re: [iproute2 PATCH v2] tc: flower: Classify packets based port ranges
From: Nambiar, Amritha @ 2018-11-10  5:05 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: stephen, netdev, jakub.kicinski, sridhar.samudrala, jhs,
	xiyou.wangcong
In-Reply-To: <20181109085143.GA2050@nanopsycho>

On 11/9/2018 12:51 AM, Jiri Pirko wrote:
> Wed, Nov 07, 2018 at 10:22:50PM CET, amritha.nambiar@intel.com wrote:
>> Added support for filtering based on port ranges.
>>
>> Example:
>> 1. Match on a port range:
>> -------------------------
>> $ tc filter add dev enp4s0 protocol ip parent ffff:\
>>  prio 1 flower ip_proto tcp dst_port range 20-30 skip_hw\
>>  action drop
>>
>> $ tc -s filter show dev enp4s0 parent ffff:
>> filter protocol ip pref 1 flower chain 0
>> filter protocol ip pref 1 flower chain 0 handle 0x1
>>  eth_type ipv4
>>  ip_proto tcp
>>  dst_port range 20-30
>>  skip_hw
>>  not_in_hw
>>        action order 1: gact action drop
>>         random type none pass val 0
>>         index 1 ref 1 bind 1 installed 85 sec used 3 sec
>>        Action statistics:
>>        Sent 460 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)
>>        backlog 0b 0p requeues 0
>>
>> 2. Match on IP address and port range:
>> --------------------------------------
>> $ tc filter add dev enp4s0 protocol ip parent ffff:\
>>  prio 1 flower dst_ip 192.168.1.1 ip_proto tcp dst_port range 100-200\
>>  skip_hw action drop
>>
>> $ tc -s filter show dev enp4s0 parent ffff:
>> filter protocol ip pref 1 flower chain 0 handle 0x2
>>  eth_type ipv4
>>  ip_proto tcp
>>  dst_ip 192.168.1.1
>>  dst_port range 100-200
>>  skip_hw
>>  not_in_hw
>>        action order 1: gact action drop
>>         random type none pass val 0
>>         index 2 ref 1 bind 1 installed 58 sec used 2 sec
>>        Action statistics:
>>        Sent 920 bytes 20 pkt (dropped 20, overlimits 0 requeues 0)
>>        backlog 0b 0p requeues 0
>>
>> v2:
>> Addressed Jiri's comment to sync output format with input
>>
>> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
>> ---
>> include/uapi/linux/pkt_cls.h |    7 ++
>> tc/f_flower.c                |  145 +++++++++++++++++++++++++++++++++++++++---
>> 2 files changed, 142 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
>> index 401d0c1..b63c3cf 100644
>> --- a/include/uapi/linux/pkt_cls.h
>> +++ b/include/uapi/linux/pkt_cls.h
>> @@ -405,6 +405,11 @@ enum {
>> 	TCA_FLOWER_KEY_UDP_SRC,		/* be16 */
>> 	TCA_FLOWER_KEY_UDP_DST,		/* be16 */
>>
>> +	TCA_FLOWER_KEY_PORT_SRC_MIN,	/* be16 */
>> +	TCA_FLOWER_KEY_PORT_SRC_MAX,	/* be16 */
>> +	TCA_FLOWER_KEY_PORT_DST_MIN,	/* be16 */
>> +	TCA_FLOWER_KEY_PORT_DST_MAX,	/* be16 */
>> +
>> 	TCA_FLOWER_FLAGS,
>> 	TCA_FLOWER_KEY_VLAN_ID,		/* be16 */
>> 	TCA_FLOWER_KEY_VLAN_PRIO,	/* u8   */
>> @@ -518,6 +523,8 @@ enum {
>> 	TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
>> };
>>
>> +#define TCA_FLOWER_MASK_FLAGS_RANGE	(1 << 0) /* Range-based match */
>> +
>> /* Match-all classifier */
>>
>> enum {
>> diff --git a/tc/f_flower.c b/tc/f_flower.c
>> index 65fca04..7724a1d 100644
>> --- a/tc/f_flower.c
>> +++ b/tc/f_flower.c
>> @@ -494,6 +494,66 @@ static int flower_parse_port(char *str, __u8 ip_proto,
>> 	return 0;
>> }
>>
>> +static int flower_port_range_attr_type(__u8 ip_proto, enum flower_endpoint type,
>> +				       __be16 *min_port_type,
>> +				       __be16 *max_port_type)
>> +{
>> +	if (ip_proto == IPPROTO_TCP || ip_proto == IPPROTO_UDP ||
>> +	    ip_proto == IPPROTO_SCTP) {
>> +		if (type == FLOWER_ENDPOINT_SRC) {
>> +			*min_port_type = TCA_FLOWER_KEY_PORT_SRC_MIN;
>> +			*max_port_type = TCA_FLOWER_KEY_PORT_SRC_MAX;
>> +		} else {
>> +			*min_port_type = TCA_FLOWER_KEY_PORT_DST_MIN;
>> +			*max_port_type = TCA_FLOWER_KEY_PORT_DST_MAX;
>> +		}
>> +	} else {
>> +		return -1;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int flower_parse_port_range(__be16 *min, __be16 *max, __u8 ip_proto,
>> +				   enum flower_endpoint endpoint,
>> +				   struct nlmsghdr *n)
>> +{
>> +	__be16 min_port_type, max_port_type;
>> +
>> +	flower_port_range_attr_type(ip_proto, endpoint, &min_port_type,
>> +				    &max_port_type);
>> +	addattr16(n, MAX_MSG, min_port_type, *min);
>> +	addattr16(n, MAX_MSG, max_port_type, *max);
>> +
>> +	return 0;
>> +}
>> +
>> +static int get_range(__be16 *min, __be16 *max, char *argv)
>> +{
>> +	char *r;
>> +
>> +	r = strchr(argv, '-');
>> +	if (r) {
>> +		*r = '\0';
>> +		if (get_be16(min, argv, 10)) {
>> +			fprintf(stderr, "invalid min range\n");
>> +			return -1;
>> +		}
>> +		if (get_be16(max, r + 1, 10)) {
>> +			fprintf(stderr, "invalid max range\n");
>> +			return -1;
>> +		}
>> +		if (htons(*max) <= htons(*min)) {
>> +			fprintf(stderr, "max value should be greater than min value\n");
>> +			return -1;
>> +		}
>> +	} else {
>> +		fprintf(stderr, "Illegal range format\n");
>> +		return -1;
>> +	}
>> +	return 0;
>> +}
>> +
>> #define TCP_FLAGS_MAX_MASK 0xfff
>>
>> static int flower_parse_tcp_flags(char *str, int flags_type, int mask_type,
>> @@ -1061,20 +1121,54 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
>> 				return -1;
>> 			}
>> 		} else if (matches(*argv, "dst_port") == 0) {
>> +			__be16 min, max;
>> +
>> 			NEXT_ARG();
>> -			ret = flower_parse_port(*argv, ip_proto,
>> -						FLOWER_ENDPOINT_DST, n);
>> -			if (ret < 0) {
>> -				fprintf(stderr, "Illegal \"dst_port\"\n");
>> -				return -1;
>> +			if (matches(*argv, "range") == 0) {
>> +				NEXT_ARG();
>> +				ret = get_range(&min, &max, *argv);
>> +				if (ret < 0)
>> +					return -1;
>> +				ret = flower_parse_port_range(&min, &max,
>> +							      ip_proto,
>> +							      FLOWER_ENDPOINT_DST,
>> +							      n);
>> +				if (ret < 0) {
>> +					fprintf(stderr, "Illegal \"dst_port range\"\n");
>> +					return -1;
>> +				}
>> +			} else {
>> +				ret = flower_parse_port(*argv, ip_proto,
>> +							FLOWER_ENDPOINT_DST, n);
>> +				if (ret < 0) {
>> +					fprintf(stderr, "Illegal \"dst_port\"\n");
>> +					return -1;
>> +				}
>> 			}
>> 		} else if (matches(*argv, "src_port") == 0) {
>> +			__be16 min, max;
>> +
>> 			NEXT_ARG();
>> -			ret = flower_parse_port(*argv, ip_proto,
>> -						FLOWER_ENDPOINT_SRC, n);
>> -			if (ret < 0) {
>> -				fprintf(stderr, "Illegal \"src_port\"\n");
>> -				return -1;
>> +			if (matches(*argv, "range") == 0) {
>> +				NEXT_ARG();
>> +				ret = get_range(&min, &max, *argv);
>> +				if (ret < 0)
>> +					return -1;
>> +				ret = flower_parse_port_range(&min, &max,
>> +							      ip_proto,
>> +							      FLOWER_ENDPOINT_SRC,
>> +							      n);
>> +				if (ret < 0) {
>> +					fprintf(stderr, "Illegal \"src_port range\"\n");
>> +					return -1;
>> +				}
>> +			} else {
>> +				ret = flower_parse_port(*argv, ip_proto,
>> +							FLOWER_ENDPOINT_SRC, n);
>> +				if (ret < 0) {
>> +					fprintf(stderr, "Illegal \"src_port\"\n");
>> +					return -1;
>> +				}
>> 			}
>> 		} else if (matches(*argv, "tcp_flags") == 0) {
>> 			NEXT_ARG();
>> @@ -1490,6 +1584,22 @@ static void flower_print_port(char *name, struct rtattr *attr)
>> 	print_hu(PRINT_ANY, name, namefrm, rta_getattr_be16(attr));
>> }
>>
>> +static void flower_print_port_range(char *name, struct rtattr *min_attr,
>> +				    struct rtattr *max_attr)
>> +{
>> +	SPRINT_BUF(namefrm);
>> +	SPRINT_BUF(out);
>> +	size_t done;
>> +
>> +	if (!min_attr || !max_attr)
>> +		return;
>> +
>> +	done = sprintf(out, "%u", rta_getattr_be16(min_attr));
>> +	sprintf(out + done, "-%u", rta_getattr_be16(max_attr));
>> +	sprintf(namefrm, "\n  %s %%s", name);
>> +	print_string(PRINT_ANY, name, namefrm, out);
>> +}
>> +
>> static void flower_print_tcp_flags(const char *name, struct rtattr *flags_attr,
>> 				   struct rtattr *mask_attr)
>> {
>> @@ -1678,6 +1788,7 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
>> 			    struct rtattr *opt, __u32 handle)
>> {
>> 	struct rtattr *tb[TCA_FLOWER_MAX + 1];
>> +	__be16 min_port_type, max_port_type;
>> 	int nl_type, nl_mask_type;
>> 	__be16 eth_type = 0;
>> 	__u8 ip_proto = 0xff;
>> @@ -1796,6 +1907,20 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
>> 	if (nl_type >= 0)
>> 		flower_print_port("src_port", tb[nl_type]);
>>
>> +	if (flower_port_range_attr_type(ip_proto, FLOWER_ENDPOINT_DST,
>> +					&min_port_type, &max_port_type)
>> +	    == 0) {
> 
> This is odd. Please convert to !fower_port....

Will fix in v3 and resubmit after the UAPI changes are accepted into
net-next.

> 
> 
> 
>> +		flower_print_port_range("dst_port range",
>> +					tb[min_port_type], tb[max_port_type]);
>> +	}
>> +
>> +	if (flower_port_range_attr_type(ip_proto, FLOWER_ENDPOINT_SRC,
>> +					&min_port_type, &max_port_type)
>> +	    == 0) {
> 
> Same here.

Will fix in v3.

> 
> 
>> +		flower_print_port_range("src_port range",
>> +					tb[min_port_type], tb[max_port_type]);
>> +	}
>> +
>> 	flower_print_tcp_flags("tcp_flags", tb[TCA_FLOWER_KEY_TCP_FLAGS],
>> 			       tb[TCA_FLOWER_KEY_TCP_FLAGS_MASK]);
>>
>>
> 
> Otherwise this looks fine.
> 

^ permalink raw reply

* [PATCH] infiniband: nes: Fix more direct skb list accesses.
From: David Miller @ 2018-11-10  5:19 UTC (permalink / raw)
  To: netdev


The following:

	skb = skb->next;
	...
	if (skb == (struct sk_buff *)queue)

is transformed into:

	skb = skb_peek_next(skb, queue);
	...
	if (!skb)

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 drivers/infiniband/hw/nes/nes_mgt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/nes/nes_mgt.c b/drivers/infiniband/hw/nes/nes_mgt.c
index e96ffff61c3a..fc0c191014e9 100644
--- a/drivers/infiniband/hw/nes/nes_mgt.c
+++ b/drivers/infiniband/hw/nes/nes_mgt.c
@@ -223,11 +223,11 @@ static struct sk_buff *nes_get_next_skb(struct nes_device *nesdev, struct nes_qp
 		}
 
 		old_skb = skb;
-		skb = skb->next;
+		skb = skb_peek_next(skb, &nesqp->pau_list);
 		skb_unlink(old_skb, &nesqp->pau_list);
 		nes_mgt_free_skb(nesdev, old_skb, PCI_DMA_TODEVICE);
 		nes_rem_ref_cm_node(nesqp->cm_node);
-		if (skb == (struct sk_buff *)&nesqp->pau_list)
+		if (!skb)
 			goto out;
 	}
 	return skb;
-- 
2.19.1

^ permalink raw reply related

* Re: [Patch net-next] net: dump more useful information in netdev_rx_csum_fault()
From: David Miller @ 2018-11-10  4:16 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev
In-Reply-To: <20181109194333.19949-1-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Fri,  9 Nov 2018 11:43:33 -0800

> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index 57f3a6fcfc1e..d8f4d55cd6c5 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -736,7 +736,7 @@ __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len)
>  	if (likely(!sum)) {
>  		if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
>  		    !skb->csum_complete_sw)
> -			netdev_rx_csum_fault(skb->dev);
> +			netdev_rx_csum_fault(skb->dev, skb);
>  	}
>  	if (!skb_shared(skb))
>  		skb->csum_valid = !sum;

Didn't you move this function into net/core/skbuff.c? :-)

Please respin.

^ 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