* Re: [PATCH iproute2 0/3] do not set IPv6-only options on IPv4 addresses
From: Andrea Claudi @ 2019-06-24 21:38 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, David Ahern
In-Reply-To: <20190624102041.25224fae@hermes.lan>
On Mon, Jun 24, 2019 at 7:21 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 24 Jun 2019 19:05:52 +0200
> Andrea Claudi <aclaudi@redhat.com> wrote:
>
> > 'home', 'nodad' and 'mngtmpaddr' options are IPv6-only, but
> > it is possible to set them on IPv4 addresses, too. This should
> > not be possible.
> >
> > Fix this adding a check on the protocol family before setting
> > the flags, and exiting with invarg() on error.
> >
> > Andrea Claudi (3):
> > ip address: do not set nodad option for IPv4 addresses
> > ip address: do not set home option for IPv4 addresses
> > ip address: do not set mngtmpaddr option for IPv4 addresses
> >
> > ip/ipaddress.c | 15 ++++++++++++---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
>
> Maybe this should be a warning, not a failure.
> A little concerned that there will be some user with a scripted setup
> that this breaks.
Hi Stephen,
I think that if a script wrongly uses some of these flags on a IPv4
address, it most probably operates on an unexpected address, since
everyone is aware that these flags are IPv6 only. In other words we
are breaking a scripted setup that is already broken.
In this case it's probably worth exiting with error and give the
author the chance to fix the script, otherwise the error can go
unnoticed.
If you prefer, I can send a v2 with warnings instead of errors, just
let me know.
Regards,
Andrea
^ permalink raw reply
* Re: [PATCH bpf-next v5 0/3] xdp: Allow lookup into devmaps before redirect
From: Toke Høiland-Jørgensen @ 2019-06-24 21:38 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Networking, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, David Miller, Jonathan Lemon
In-Reply-To: <CAEf4BzZPb2YsrvOfshJAboY9KE3dCa_FZsTkxvQyPquzDChz+w@mail.gmail.com>
Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
> On Mon, Jun 24, 2019 at 12:38 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
>>
>> > On Sat, Jun 22, 2019 at 7:19 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>> >>
>> >> When using the bpf_redirect_map() helper to redirect packets from XDP, the eBPF
>> >> program cannot currently know whether the redirect will succeed, which makes it
>> >> impossible to gracefully handle errors. To properly fix this will probably
>> >> require deeper changes to the way TX resources are allocated, but one thing that
>> >> is fairly straight forward to fix is to allow lookups into devmaps, so programs
>> >> can at least know when a redirect is *guaranteed* to fail because there is no
>> >> entry in the map. Currently, programs work around this by keeping a shadow map
>> >> of another type which indicates whether a map index is valid.
>> >>
>> >> This series contains two changes that are complementary ways to fix this issue:
>> >>
>> >> - Moving the map lookup into the bpf_redirect_map() helper (and caching the
>> >> result), so the helper can return an error if no value is found in the map.
>> >> This includes a refactoring of the devmap and cpumap code to not care about
>> >> the index on enqueue.
>> >>
>> >> - Allowing regular lookups into devmaps from eBPF programs, using the read-only
>> >> flag to make sure they don't change the values.
>> >>
>> >> The performance impact of the series is negligible, in the sense that I cannot
>> >> measure it because the variance between test runs is higher than the difference
>> >> pre/post series.
>> >>
>> >> Changelog:
>> >>
>> >> v5:
>> >> - Rebase on latest bpf-next.
>> >> - Update documentation for bpf_redirect_map() with the new meaning of flags.
>> >>
>> >> v4:
>> >> - Fix a few nits from Andrii
>> >> - Lose the #defines in bpf.h and just compare the flags argument directly to
>> >> XDP_TX in bpf_xdp_redirect_map().
>> >>
>> >> v3:
>> >> - Adopt Jonathan's idea of using the lower two bits of the flag value as the
>> >> return code.
>> >> - Always do the lookup, and cache the result for use in xdp_do_redirect(); to
>> >> achieve this, refactor the devmap and cpumap code to get rid the bitmap for
>> >> selecting which devices to flush.
>> >>
>> >> v2:
>> >> - For patch 1, make it clear that the change works for any map type.
>> >> - For patch 2, just use the new BPF_F_RDONLY_PROG flag to make the return
>> >> value read-only.
>> >>
>> >> ---
>> >>
>> >> Toke Høiland-Jørgensen (3):
>> >> devmap/cpumap: Use flush list instead of bitmap
>> >> bpf_xdp_redirect_map: Perform map lookup in eBPF helper
>> >> devmap: Allow map lookups from eBPF
>> >>
>> >>
>> >> include/linux/filter.h | 1
>> >> include/uapi/linux/bpf.h | 7 ++-
>> >> kernel/bpf/cpumap.c | 106 ++++++++++++++++++++-----------------------
>> >> kernel/bpf/devmap.c | 113 ++++++++++++++++++++++------------------------
>> >> kernel/bpf/verifier.c | 7 +--
>> >> net/core/filter.c | 29 +++++-------
>> >> 6 files changed, 123 insertions(+), 140 deletions(-)
>> >>
>> >
>> >
>> > Looks like you forgot to add my Acked-by's for your patches?
>>
>> Ah yes, did not carry those forward for the individual patches, my
>> apologies. Could you perhaps be persuaded to send a new one (I believe a
>> response to the cover letter acking the whole series would suffice)?
>> I'll make sure to add the carrying forward of acks into my workflow in
>> the future :)
>
> I don't think patchworks captures ack's from cover letter, but let's
> give it a go:
>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
Thanks!
-Toke
^ permalink raw reply
* Re: [PATCH net-next v5 2/7] etf: Add skip_sock_check
From: Jakub Kicinski @ 2019-06-24 21:40 UTC (permalink / raw)
To: Vedang Patel
Cc: netdev, jeffrey.t.kirsher, davem, jhs, xiyou.wangcong, jiri,
intel-wired-lan, vinicius.gomes, l, m-karicheri2, sergei.shtylyov,
eric.dumazet, aaron.f.brown
In-Reply-To: <1561138108-12943-3-git-send-email-vedang.patel@intel.com>
On Fri, 21 Jun 2019 10:28:23 -0700, Vedang Patel wrote:
> diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
> index 8b2f993cbb77..409d1616472d 100644
> --- a/include/uapi/linux/pkt_sched.h
> +++ b/include/uapi/linux/pkt_sched.h
> @@ -990,6 +990,7 @@ struct tc_etf_qopt {
> __u32 flags;
> #define TC_ETF_DEADLINE_MODE_ON BIT(0)
> #define TC_ETF_OFFLOAD_ON BIT(1)
> +#define TC_ETF_SKIP_SOCK_CHECK BIT(2)
> };
>
> enum {
I think build bot complained about the code not building on 32bit.
When you respin could you include a patch to remove the uses of BIT()
in UAPI? See: https://www.spinics.net/lists/netdev/msg579344.html
^ permalink raw reply
* Re: [PATCH net-next 13/18] ionic: Add initial ethtool support
From: Shannon Nelson @ 2019-06-24 21:44 UTC (permalink / raw)
To: Michal Kubecek, netdev
In-Reply-To: <20190624072604.GA27240@unicorn.suse.cz>
On 6/24/19 12:26 AM, Michal Kubecek wrote:
> On Fri, Jun 21, 2019 at 03:30:20PM -0700, Shannon Nelson wrote:
>> On 6/20/19 7:32 PM, Michal Kubecek wrote:
>>> On Thu, Jun 20, 2019 at 01:24:19PM -0700, Shannon Nelson wrote:
>>> +
>>> + if (ch->combined_count > lif->ionic->ntxqs_per_lif)
>>> + return -EINVAL;
>>> This has been already checked in ethtool_set_channels().
>> That's what I get for copying from an existing driver. I'll check those and
>> clean them up.
> The checks in general code were only added recently so most drivers
> probably still have their own checks.
>
That would be the reason. Thanks, I'll clean those up.
sln
^ permalink raw reply
* Re: [PATCH net-next 01/18] ionic: Add basic framework for IONIC Network device driver
From: Shannon Nelson @ 2019-06-24 21:46 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev
In-Reply-To: <20190624130327.2b16d149@cakuba.netronome.com>
On 6/24/19 1:03 PM, Jakub Kicinski wrote:
> On Thu, 20 Jun 2019 13:24:07 -0700, Shannon Nelson wrote:
>> diff --git a/Documentation/networking/device_drivers/pensando/ionic.rst b/Documentation/networking/device_drivers/pensando/ionic.rst
>> new file mode 100644
>> index 000000000000..84bdf682052b
>> --- /dev/null
>> +++ b/Documentation/networking/device_drivers/pensando/ionic.rst
>> @@ -0,0 +1,75 @@
>> +.. SPDX-License-Identifier: GPL-2.0+
>> +
>> +==========================================================
>> +Linux* Driver for the Pensando(R) Ethernet adapter family
>> +==========================================================
>> +
>> +Pensando Linux Ethernet driver.
>> +Copyright(c) 2019 Pensando Systems, Inc
>> +
>> +Contents
>> +========
>> +
>> +- Identifying the Adapter
>> +- Special Features
>> +- Support
>> +
>> +
> nit: all instances of multiple empty lines in the docs look a bit
> unnecessary
Yep, looks like I missed a couple. I'll check those again.
sln
^ permalink raw reply
* Re: [RFC PATCH net-next 1/1] tc-testing: Scapy plugin and JSON verification for tdc
From: Alexander Aring @ 2019-06-24 21:47 UTC (permalink / raw)
To: Lucas Bates
Cc: netdev, davem, jhs, kernel, xiyou.wangcong, jiri, mleitner,
vladbu, dcaratti
In-Reply-To: <1560133232-17880-1-git-send-email-lucasb@mojatatu.com>
Hi Lucas,
On Sun, Jun 09, 2019 at 10:20:32PM -0400, Lucas Bates wrote:
> This is a draft version of two new tdc features.
>
> First, the scapy plugin. This requires the scapy Python module
> installed on the system (plugin was tested against v2.4.2).
> The intent is to install a given rule (as the command under test)
> and then generate packets to create statistics for that rule.
> The stats are checked in the verify phase.
>
> A new "scapy" entry appears in the test cases, which currently
> have three requirements: the source interface for the packets,
> the number of packets to be sent, and a string that is processed
> by scapy's eval() function to construct the packets.
eval() is not a function of scapy, it is a python thing.
You need to be careful with that, people can introduce weird code. Since
everything is here under review I think it is okay... so far we don't
introduce a tdc restful API for that.
>
> Limitations: For now, only one type of packet can be crafted
> per test case. Also, knowledge of scapy's syntax is required.
>
Why not add a list of eval() strings and send them out in order?
> Secondly, we add JSON processing as a method of performing the
> verification stage. Each test case can now have a "matchPattern"
> or "matchJSON" field which governs the method tdc will use to
> process the results. The intent is to make it easier to handle
> the verify stage by not requiring complex regular expressions
>
> matchJSON has two fields, path and value. Path is a list of
> strings and integers which indicate the path through the nested
> JSON data - an asterisk is also acceptable in place of
> a number if the specific index of a list is unknown.
>
Can you provide an example?
> This structure may not be the best method of handling JSON
> verification - suggestions have been made that include using a
> third party module to process the JSON, but that creates an
> external dependency for tdc.
>
> To try the sample tests in this patch:
>
> 1) Ensure nsPlugin and scapyPlugin are linked in the plugins/
> subdirectory
> 2) Run:
> sudo ./tdc.py -f tc-tests/actions/scapy-example.json -n
>
> The second test is designed to fail.
>
> Comments and discussion are encouraged!
Can you please split these patches according to your commit message.
1. JSON processing (new core feature?)
2. scapy plugin (should not touch core code)
3. scapy example
- Alex
^ permalink raw reply
* Re: [PATCH net-next 02/18] ionic: Add hardware init and device commands
From: Shannon Nelson @ 2019-06-24 21:50 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Andrew Lunn, netdev
In-Reply-To: <20190624131304.78c1a4a9@cakuba.netronome.com>
On 6/24/19 1:13 PM, Jakub Kicinski wrote:
> On Fri, 21 Jun 2019 15:22:22 -0700, Shannon Nelson wrote:
>>>> +static int identity_show(struct seq_file *seq, void *v)
>>>> +{
>>>> + struct ionic *ionic = seq->private;
>>>> + struct identity *ident = &ionic->ident;
>>>> + struct ionic_dev *idev = &ionic->idev;
>>>> +
>>>> + seq_printf(seq, "asic_type: 0x%x\n", idev->dev_info.asic_type);
>>>> + seq_printf(seq, "asic_rev: 0x%x\n", idev->dev_info.asic_rev);
>>>> + seq_printf(seq, "serial_num: %s\n", idev->dev_info.serial_num);
>>>> + seq_printf(seq, "fw_version: %s\n", idev->dev_info.fw_version);
>>>> + seq_printf(seq, "fw_status: 0x%x\n",
>>>> + ioread8(&idev->dev_info_regs->fw_status));
>>>> + seq_printf(seq, "fw_heartbeat: 0x%x\n",
>>>> + ioread32(&idev->dev_info_regs->fw_heartbeat));
>>> devlink just gained a much more flexible version of ethtool -i. Please
>>> remove all this and use that.
>> Yes, we intend to add a devlink interface, it just isn't in this first
>> patchset, which is already plenty big.
> Please take this out of your patch set, we can't be expected to merge
> debugfs implementation of what has proper APIs :/
Got it.
sln
^ permalink raw reply
* Re: [PATCH iproute2 0/3] do not set IPv6-only options on IPv4 addresses
From: David Ahern @ 2019-06-24 21:51 UTC (permalink / raw)
To: Andrea Claudi, Stephen Hemminger; +Cc: netdev, David Ahern
In-Reply-To: <CAPpH65wG9OXhEnXd2LrL0wQc9P4G7MKQjQ4bUTHN1CVqAc6bMg@mail.gmail.com>
On 6/24/19 3:38 PM, Andrea Claudi wrote:
> I think that if a script wrongly uses some of these flags on a IPv4
> address, it most probably operates on an unexpected address, since
> everyone is aware that these flags are IPv6 only. In other words we
> are breaking a scripted setup that is already broken.
> In this case it's probably worth exiting with error and give the
> author the chance to fix the script, otherwise the error can go
> unnoticed.
>
> If you prefer, I can send a v2 with warnings instead of errors, just
> let me know.
Recent changes for strict mode have shown people do interesting things
with scripts and like the silent "ignores". :-(
^ permalink raw reply
* Re: [PATCH bpf-next] bpftool: Add BPF_F_QUERY_EFFECTIVE support in bpftool cgroup [show|tree]
From: Jakub Kicinski @ 2019-06-24 21:51 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Takshak Chahande, netdev, ast, rdna, kernel-team,
Stanislav Fomichev
In-Reply-To: <6fe292ee-fff0-119c-8524-e25783901167@iogearbox.net>
On Mon, 24 Jun 2019 16:22:25 +0200, Daniel Borkmann wrote:
> On 06/22/2019 12:33 AM, Takshak Chahande wrote:
> > With different bpf attach_flags available to attach bpf programs specially
> > with BPF_F_ALLOW_OVERRIDE and BPF_F_ALLOW_MULTI, the list of effective
> > bpf-programs available to any sub-cgroups really needs to be available for
> > easy debugging.
> >
> > Using BPF_F_QUERY_EFFECTIVE flag, one can get the list of not only attached
> > bpf-programs to a cgroup but also the inherited ones from parent cgroup.
> >
> > So "-e" option is introduced to use BPF_F_QUERY_EFFECTIVE query flag here to
> > list all the effective bpf-programs available for execution at a specified
> > cgroup.
> >
> > Reused modified test program test_cgroup_attach from tools/testing/selftests/bpf:
> > # ./test_cgroup_attach
> >
> > With old bpftool (without -e option):
> >
> > # bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/
> > ID AttachType AttachFlags Name
> > 271 egress multi pkt_cntr_1
> > 272 egress multi pkt_cntr_2
> >
> > Attached new program pkt_cntr_4 in cg2 gives following:
> >
> > # bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/cg2
> > ID AttachType AttachFlags Name
> > 273 egress override pkt_cntr_4
> >
> > And with new "-e" option it shows all effective programs for cg2:
> >
> > # bpftool -e cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/cg2
> > ID AttachType AttachFlags Name
> > 273 egress override pkt_cntr_4
> > 271 egress override pkt_cntr_1
> > 272 egress override pkt_cntr_2
> >
> > Signed-off-by: Takshak Chahande <ctakshak@fb.com>
> > Acked-by: Andrey Ignatov <rdna@fb.com>
>
> Applied, thanks!
This is a cgroup-specific flag, right? It should be a parameter
to cgroup show, not a global flag. Can we please drop this patch
from the tree?
^ permalink raw reply
* Re: [PATCH v3 net] af_packet: Block execution of tasks waiting for transmit to complete in AF_PACKET
From: Neil Horman @ 2019-06-24 21:51 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, Matteo Croce, David S. Miller
In-Reply-To: <CAF=yD-JE9DEbmh6hJEN=DEdc+SCz_5Lv74mngPBuv=4nNH=zxQ@mail.gmail.com>
On Mon, Jun 24, 2019 at 02:08:43PM -0400, Willem de Bruijn wrote:
> On Sun, Jun 23, 2019 at 8:46 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > When an application is run that:
> > a) Sets its scheduler to be SCHED_FIFO
> > and
> > b) Opens a memory mapped AF_PACKET socket, and sends frames with the
> > MSG_DONTWAIT flag cleared, its possible for the application to hang
> > forever in the kernel. This occurs because when waiting, the code in
> > tpacket_snd calls schedule, which under normal circumstances allows
> > other tasks to run, including ksoftirqd, which in some cases is
> > responsible for freeing the transmitted skb (which in AF_PACKET calls a
> > destructor that flips the status bit of the transmitted frame back to
> > available, allowing the transmitting task to complete).
> >
> > However, when the calling application is SCHED_FIFO, its priority is
> > such that the schedule call immediately places the task back on the cpu,
> > preventing ksoftirqd from freeing the skb, which in turn prevents the
> > transmitting task from detecting that the transmission is complete.
> >
> > We can fix this by converting the schedule call to a completion
> > mechanism. By using a completion queue, we force the calling task, when
> > it detects there are no more frames to send, to schedule itself off the
> > cpu until such time as the last transmitted skb is freed, allowing
> > forward progress to be made.
> >
> > Tested by myself and the reporter, with good results
> >
> > Appies to the net tree
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > Reported-by: Matteo Croce <mcroce@redhat.com>
> > CC: "David S. Miller" <davem@davemloft.net>
> > CC: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> >
> > Change Notes:
> >
> > V1->V2:
> > Enhance the sleep logic to support being interruptible and
> > allowing for honoring to SK_SNDTIMEO (Willem de Bruijn)
> >
> > V2->V3:
> > Rearrage the point at which we wait for the completion queue, to
> > avoid needing to check for ph/skb being null at the end of the loop.
> > Also move the complete call to the skb destructor to avoid needing to
> > modify __packet_set_status. Also gate calling complete on
> > packet_read_pending returning zero to avoid multiple calls to complete.
> > (Willem de Bruijn)
> >
> > Move timeo computation within loop, to re-fetch the socket
> > timeout since we also use the timeo variable to record the return code
> > from the wait_for_complete call (Neil Horman)
> > ---
> > net/packet/af_packet.c | 59 +++++++++++++++++++++++++++++++++++++-----
> > net/packet/internal.h | 2 ++
> > 2 files changed, 55 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> > index a29d66da7394..5c48bb7a4fa5 100644
> > --- a/net/packet/af_packet.c
> > +++ b/net/packet/af_packet.c
> > @@ -380,7 +380,6 @@ static void __packet_set_status(struct packet_sock *po, void *frame, int status)
> > WARN(1, "TPACKET version not supported.\n");
> > BUG();
> > }
> > -
>
> Unrelated to this feature
>
Agreed.
> > smp_wmb();
><snip>
>
> > ph = packet_current_frame(po, &po->tx_ring,
> > TP_STATUS_SEND_REQUEST);
> > - if (unlikely(ph == NULL)) {
> > - if (need_wait && need_resched())
> > - schedule();
> > - continue;
>
> Why not keep the test whether the process needs to wait exactly here (A)?
>
As I said in the changelog, I think it makes the code more readable, to
understand that you are waiting for an event to complete after you send the
frame.
> Then no need for packet_next_frame.
>
Thats fair. I still think waiting at the bottom of the loop is more clear, but
it does save a function, so I'll agree to this.
> > - }
> > +
> > + if (unlikely(ph == NULL))
> > + break;
> >
> > skb = NULL;
> > tp_len = tpacket_parse_header(po, ph, size_max, &data);
> > +
>
> Again
>
> > if (tp_len < 0)
> > goto tpacket_error;
> >
> > @@ -2720,6 +2732,21 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> >
> > skb->destructor = tpacket_destruct_skb;
> > __packet_set_status(po, ph, TP_STATUS_SENDING);
> > +
> > + /*
> > + * If we need to wait and we've sent the last frame pending
> > + * transmission in the mmaped buffer, flag that we need to wait
> > + * on those frames to get freed via tpacket_destruct_skb. This
> > + * flag indicates that tpacket_destruct_skb should call complete
> > + * when the packet_pending count reaches zero, and that we need
> > + * to call wait_on_complete_interruptible_timeout below, to make
> > + * sure we pick up the result of that completion
> > + */
> > + if (need_wait && !packet_next_frame(po, &po->tx_ring, TP_STATUS_SEND_REQUEST)) {
> > + po->wait_on_complete = 1;
> > + timeo = sock_sndtimeo(&po->sk, msg->msg_flags & MSG_DONTWAIT);
>
> This resets timeout on every loop. should only set above the loop once.
>
I explained exactly why I did that in the change log. Its because I reuse the
timeout variable to get the return value of the wait_for_complete call.
Otherwise I need to add additional data to the stack, which I don't want to do.
Sock_sndtimeo is an inline function and really doesn't add any overhead to this
path, so I see no reason not to reuse the variable.
> Also, please limit the comments in the code (also below). If every
> patch would add this many lines of comment, the file would be
> enormous. OTOH, it's great to be this explanatory in the git commit,
> which is easily reached for any line with git blame.
>
> > + }
> > +
> > packet_inc_pending(&po->tx_ring);
> >
> > status = TP_STATUS_SEND_REQUEST;
> > @@ -2728,6 +2755,11 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> > err = net_xmit_errno(err);
> > if (err && __packet_get_status(po, ph) ==
> > TP_STATUS_AVAILABLE) {
> > + /* re-init completion queue to avoid subsequent fallthrough
> > + * on a future thread calling wait_on_complete_interruptible_timeout
> > + */
> > + po->wait_on_complete = 0;
>
> If setting where sleeping, no need for resetting if a failure happens
> between those blocks.
>
> > + init_completion(&po->skb_completion);
>
> no need to reinit between each use?
>
I explained exactly why I did this in the comment above. We have to set
wait_for_complete prior to calling transmit, so as to ensure that we call
wait_for_completion before we exit the loop. However, in this error case, we
exit the loop prior to calling wait_for_complete, so we need to reset the
completion variable and the wait_for_complete flag. Otherwise we will be in a
case where, on the next entrace to this loop we will have a completion variable
with completion->done > 0, meaning the next wait will be a fall through case,
which we don't want.
> > /* skb was destructed already */
> > skb = NULL;
> > goto out_status;
> > @@ -2740,6 +2772,20 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> > }
> > packet_increment_head(&po->tx_ring);
> > len_sum += tp_len;
> > +
> > + if (po->wait_on_complete) {
> > + timeo = wait_for_completion_interruptible_timeout(&po->skb_completion, timeo);
> > + po->wait_on_complete = 0;
>
> I was going to argue for clearing in tpacket_destruct_skb. But then we
> would have to separate clear on timeout instead of signal, too.
>
> po->wait_on_complete = 1;
> timeo = wait_for_completion...
> po->wait_on_complete = 0;
>
Also, we would have a race condition, since the destructor may be called from
softirq context (the first cause of the bug I'm fixing here), and so if the
packet is freed prior to us checking wait_for_complete in tpacket_snd, we will
be in the above situation again, exiting the loop with a completion variable in
an improper state.
> as the previous version had is fine, as long as the compiler does not
> "optimize" away an assignment. The function call will avoid reordering
> by the cpu, at least. Probably requires WRITE_ONCE/READ_ONCE.
>
> > + if (!timeo) {
> > + /* We timed out, break out and notify userspace */
> > + err = -ETIMEDOUT;
> > + goto out_status;
>
> goto out_put, there is no active ph or skb here
>
Yes, good catch.
> > + } else if (timeo == -ERESTARTSYS) {
> > + err = -ERESTARTSYS;
> > + goto out_status;
> > + }
> > + }
> > +
> > } while (likely((ph != NULL) ||
> > /* Note: packet_read_pending() might be slow if we have
> > * to call it as it's per_cpu variable, but in fast-path
> > @@ -3207,6 +3253,7 @@ static int packet_create(struct net *net, struct socket *sock, int protocol,
> > sock_init_data(sock, sk);
> >
> > po = pkt_sk(sk);
> > + init_completion(&po->skb_completion);
> > sk->sk_family = PF_PACKET;
> > po->num = proto;
> > po->xmit = dev_queue_xmit;
>
> This is basically replacing a busy-wait with schedule() with sleeping
> using wait_for_completion_interruptible_timeout. My main question is
> does this really need to move control flow around and add
> packet_next_frame? If not, especially for net, the shortest, simplest
> change is preferable.
>
Its not replacing a busy wait at all, its replacing a non-blocking schedule with
a blocking schedule (via completion queues). As for control flow, Im not sure I
why you are bound to the existing control flow, and given that we already have
packet_previous_frame, I didn't see anything egregious about adding
packet_next_frame as well, but since you've seen a way to eliminate it, I'm ok
with it.
Neil
^ permalink raw reply
* Re: [PATCH RFC net-next 5/5] net: dsa: mt7530: Add mediatek,ephy-handle to isolate external phy
From: Andrew Lunn @ 2019-06-24 21:52 UTC (permalink / raw)
To: René van Dorst
Cc: sean.wang, f.fainelli, linux, davem, matthias.bgg, vivien.didelot,
frank-w, netdev, linux-mediatek, linux-mips
In-Reply-To: <20190624145251.4849-6-opensource@vdorst.com>
> +static int mt7530_isolate_ephy(struct dsa_switch *ds,
> + struct device_node *ephy_node)
> +{
> + struct phy_device *phydev = of_phy_find_device(ephy_node);
> + int ret;
> +
> + if (!phydev)
> + return 0;
> +
> + ret = phy_modify(phydev, MII_BMCR, 0, (BMCR_ISOLATE | BMCR_PDOWN));
genphy_suspend() does what you want.
> + if (ret)
> + dev_err(ds->dev, "Failed to put phy %s in isolation mode!\n",
> + ephy_node->full_name);
> + else
> + dev_info(ds->dev, "Phy %s in isolation mode!\n",
> + ephy_node->full_name);
No need to clog up the system with yet more kernel messages.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 00/18] Add ionic driver
From: David Miller @ 2019-06-24 21:53 UTC (permalink / raw)
To: jakub.kicinski; +Cc: snelson, netdev, andrew
In-Reply-To: <20190624131952.0b90206e@cakuba.netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 24 Jun 2019 13:19:52 -0700
> On Thu, 20 Jun 2019 13:24:06 -0700, Shannon Nelson wrote:
>> 28 files changed, 9970 insertions(+)
>
> Dave, could we consider setting a LoC limit for series and patches?
> I know this is a new driver, but there's gotta be a way to split
> this up more, even if it's painful for the submitter :S
>
> All the debugfs stuff shouldn't be necessary in the first version,
> just looking at first 2 patches...
I hear what you're saying. But I have to balance this with the concern
for creating a barrier for entry to submitting new drivers.
However, looking from another perspective you are right that review
burdon is not purely on a number of patches level, but rather the
product of number of patches and lines per patch and thus LoC.
I'd hate to specify a hard nuber and would rather try to apply
judgment onto individual submissions and deal with it on a case
by case basis.
If someone thinks a submission is too large, anyone can just say that
and we'll see what happens.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 01/18] ionic: Add basic framework for IONIC Network device driver
From: Shannon Nelson @ 2019-06-24 21:54 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Andrew Lunn, netdev
In-Reply-To: <20190624130759.3d413c26@cakuba.netronome.com>
On 6/24/19 1:07 PM, Jakub Kicinski wrote:
> On Fri, 21 Jun 2019 15:13:31 -0700, Shannon Nelson wrote:
>>>> +#define DRV_VERSION "0.11.0-k"
>>> DRV_VERSION is pretty useless. What you really want to know is the
>>> kernel git tree and commit. The big distributions might backport this
>>> version of the driver back to the old kernel with a million
>>> patches. At which point 0.11.0-k tells you nothing much.
>> Yes, any version numbering thing from the big distros is put into
>> question, but I find this number useful to me for tracking what has been
>> put into the upstream kernel. This plus the full kernel version gives
>> me a pretty good idea of what I'm looking at.
> Still, we strongly encourage ditching the driver version.
> It encourages upstream first development model among other benefits.
< insert typical whining about internal vendor needs that have
nothing to do with upstream practices :-) >
Sure.
sln
^ permalink raw reply
* Re: [PATCH iproute2] iproute: Pass RTM_F_CLONED on dump to fetch cached routes to be flushed
From: David Ahern @ 2019-06-24 21:55 UTC (permalink / raw)
To: Stefano Brivio, Stephen Hemminger
Cc: David Miller, Jianlin Shi, Wei Wang, Martin KaFai Lau,
Eric Dumazet, Matti Vaittinen, netdev
In-Reply-To: <7ae318a8b632c216df95362524cd4bb5f4f1f537.1560561439.git.sbrivio@redhat.com>
On 6/14/19 7:33 PM, Stefano Brivio wrote:
> diff --git a/ip/iproute.c b/ip/iproute.c
> index 2b3dcc5dbd53..192442b42062 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -1602,6 +1602,16 @@ static int save_route_prep(void)
> return 0;
> }
>
> +static int iproute_flush_flags(struct nlmsghdr *nlh, int reqlen)
rename that to iproute_flush_filter to be consistent with
iproute_dump_filter.
Actually, why can't the flush code use iproute_dump_filter?
> +{
> + struct rtmsg *rtm = NLMSG_DATA(nlh);
> +
> + if (filter.cloned)
> + rtm->rtm_flags |= RTM_F_CLONED;
> +
> + return 0;
> +}
> +
> static int iproute_flush(int family, rtnl_filter_t filter_fn)
> {
> time_t start = time(0);
> @@ -1624,7 +1634,7 @@ static int iproute_flush(int family, rtnl_filter_t filter_fn)
> filter.flushe = sizeof(flushb);
>
> for (;;) {
> - if (rtnl_routedump_req(&rth, family, NULL) < 0) {
> + if (rtnl_routedump_req(&rth, family, iproute_flush_flags) < 0) {
> perror("Cannot send dump request");
> return -2;
> }
>
^ permalink raw reply
* Re: [PATCH v3 net-next 0/4] cxgb4: Reference count MPS TCAM entries within a PF
From: David Miller @ 2019-06-24 21:56 UTC (permalink / raw)
To: rajur; +Cc: netdev, nirranjan, dt
In-Reply-To: <20190624173535.12572-1-rajur@chelsio.com>
From: Raju Rangoju <rajur@chelsio.com>
Date: Mon, 24 Jun 2019 23:05:31 +0530
> Firmware reference counts the MPS TCAM entries by PF and VF,
> but it does not do it for usage within a PF or VF. This patch
> adds the support to track MPS TCAM entries within a PF.
>
> v2->v3:
> Fixed the compiler errors due to incorrect patch
> Also, removed the new blank line at EOF
> v1->v2:
> Use refcount_t type instead of atomic_t for mps reference count
Series applied, th anks.
^ permalink raw reply
* Re: [PATCH RFC net-next 4/5] dt-bindings: net: dsa: mt7530: Add mediatek,ephy-handle to isolate ext. phy
From: Florian Fainelli @ 2019-06-24 21:56 UTC (permalink / raw)
To: René van Dorst, sean.wang, linux, davem, matthias.bgg,
andrew, vivien.didelot
Cc: frank-w, netdev, linux-mediatek, linux-mips
In-Reply-To: <20190624145251.4849-5-opensource@vdorst.com>
On 6/24/19 7:52 AM, René van Dorst wrote:
> On some platforum the external phy can only interface to the port 5 of the
> switch because the RGMII TX and RX lines are swapped. But it still can be
> useful to use the internal phy of the switch to act as a WAN port which
> connectes to the 2nd GMAC. This gives WAN port dedicated bandwidth to
> the SOC. This increases the LAN and WAN routing.
>
> By adding the optional property mediatek,ephy-handle, the external phy
> is put in isolation mode when internal phy is connected to 2nd GMAC via
> phy-handle property.
Most platforms we have seen so far implement this logic with a mdio-mux,
can you see if that is possible here? stmmac has plenty of examples like
those.
--
Florian
^ permalink raw reply
* Re: [PATCH rdma-next v1 00/12] DEVX asynchronous events
From: Saeed Mahameed @ 2019-06-24 21:57 UTC (permalink / raw)
To: leon@kernel.org
Cc: Jason Gunthorpe, Yishai Hadas, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, dledford@redhat.com
In-Reply-To: <20190619044557.GA11611@mtr-leonro.mtl.com>
On Wed, 2019-06-19 at 07:45 +0300, Leon Romanovsky wrote:
> On Tue, Jun 18, 2019 at 06:51:45PM +0000, Saeed Mahameed wrote:
> > On Tue, 2019-06-18 at 20:15 +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@mellanox.com>
> > >
> > > Changelog:
> > > v0 -> v1:
> >
> > Normally 1st submission is V1 and 2nd is V2.
> > so this should have been v1->v2.
>
> "Normally" depends on the language you are using. In C, everything
> starts from 0, including version of patches :).
>
You are wrong:
quoting: https://kernelnewbies.org/PatchTipsAndTricks
"For example, if you're sending the second revision of a patch, you
should use [PATCH v2]."
now don't tell me that second revision is actually 3rd revision or 1st
is 2nd :)..
> > For mlx5-next patches:
> >
> > Acked-by: Saeed Mahameed <saeedm@mellanox.com>
>
> Thanks
^ permalink raw reply
* [PATCH] bpf: Allow bpf_skb_event_output for a few prog types
From: allanzhang @ 2019-06-24 21:58 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, David S. Miller, netdev, bpf, linux-kernel
Cc: allanzhang
In-Reply-To: <20190624215824.118783-1-allanzhang@google.com>
Software event output is only enabled by a few prog types right now (TC,
LWT out, XDP, sockops). Many other skb based prog types need
bpf_skb_event_output to produce software event.
Added socket_filter, cg_skb, sk_skb prog types to generate sw event.
Test bpf code is generated from code snippet:
struct TMP {
uint64_t tmp;
} tt;
tt.tmp = 5;
bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
&tt, sizeof(tt));
return 1;
the bpf assembly from llvm is:
0: b7 02 00 00 05 00 00 00 r2 = 5
1: 7b 2a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r2
2: bf a4 00 00 00 00 00 00 r4 = r10
3: 07 04 00 00 f8 ff ff ff r4 += -8
4: 18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0ll
6: b7 03 00 00 00 00 00 00 r3 = 0
7: b7 05 00 00 08 00 00 00 r5 = 8
8: 85 00 00 00 19 00 00 00 call 25
9: b7 00 00 00 01 00 00 00 r0 = 1
10: 95 00 00 00 00 00 00 00 exit
Patch 1 is enabling code.
Patch 2 is fullly covered selftest code.
Signed-off-by: allanzhang <allanzhang@google.com>
---
net/core/filter.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/core/filter.c b/net/core/filter.c
index 2014d76e0d2a..b75fcf412628 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5958,6 +5958,8 @@ sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_get_socket_cookie_proto;
case BPF_FUNC_get_socket_uid:
return &bpf_get_socket_uid_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_skb_event_output_proto;
default:
return bpf_base_func_proto(func_id);
}
@@ -5978,6 +5980,8 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sk_storage_get_proto;
case BPF_FUNC_sk_storage_delete:
return &bpf_sk_storage_delete_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_skb_event_output_proto;
#ifdef CONFIG_SOCK_CGROUP_DATA
case BPF_FUNC_skb_cgroup_id:
return &bpf_skb_cgroup_id_proto;
@@ -6226,6 +6230,8 @@ sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sk_redirect_map_proto;
case BPF_FUNC_sk_redirect_hash:
return &bpf_sk_redirect_hash_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_skb_event_output_proto;
#ifdef CONFIG_INET
case BPF_FUNC_sk_lookup_tcp:
return &bpf_sk_lookup_tcp_proto;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH] bpf: Allow bpf_skb_event_output for a few prog types
From: allanzhang @ 2019-06-24 21:58 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, David S. Miller, netdev, bpf, linux-kernel
Cc: allanzhang
In-Reply-To: <20190624215824.118783-1-allanzhang@google.com>
Software event output is only enabled by a few prog types right now (TC,
LWT out, XDP, sockops). Many other skb based prog types need
bpf_skb_event_output to produce software event.
Added socket_filter, cg_skb, sk_skb prog types to generate sw event.
Test bpf code is generated from code snippet:
struct TMP {
uint64_t tmp;
} tt;
tt.tmp = 5;
bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
&tt, sizeof(tt));
return 1;
the bpf assembly from llvm is:
0: b7 02 00 00 05 00 00 00 r2 = 5
1: 7b 2a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r2
2: bf a4 00 00 00 00 00 00 r4 = r10
3: 07 04 00 00 f8 ff ff ff r4 += -8
4: 18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0ll
6: b7 03 00 00 00 00 00 00 r3 = 0
7: b7 05 00 00 08 00 00 00 r5 = 8
8: 85 00 00 00 19 00 00 00 call 25
9: b7 00 00 00 01 00 00 00 r0 = 1
10: 95 00 00 00 00 00 00 00 exit
Patch 1 is enabling code.
Patch 2 is fullly covered selftest code.
Signed-off-by: allanzhang <allanzhang@google.com>
---
tools/testing/selftests/bpf/test_verifier.c | 38 +++++++-
.../selftests/bpf/verifier/event_output.c | 94 +++++++++++++++++++
2 files changed, 130 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/bpf/verifier/event_output.c
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index c5514daf8865..45ce9dd4323f 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -1,10 +1,13 @@
-// SPDX-License-Identifier: GPL-2.0-only
/*
* Testsuite for eBPF verifier
*
* Copyright (c) 2014 PLUMgrid, http://plumgrid.com
* Copyright (c) 2017 Facebook
* Copyright (c) 2018 Covalent IO, Inc. http://covalent.io
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
*/
#include <endian.h>
@@ -50,7 +53,7 @@
#define MAX_INSNS BPF_MAXINSNS
#define MAX_TEST_INSNS 1000000
#define MAX_FIXUPS 8
-#define MAX_NR_MAPS 18
+#define MAX_NR_MAPS 19
#define MAX_TEST_RUNS 8
#define POINTER_VALUE 0xcafe4all
#define TEST_DATA_LEN 64
@@ -84,6 +87,7 @@ struct bpf_test {
int fixup_map_array_wo[MAX_FIXUPS];
int fixup_map_array_small[MAX_FIXUPS];
int fixup_sk_storage_map[MAX_FIXUPS];
+ int fixup_map_event_output[MAX_FIXUPS];
const char *errstr;
const char *errstr_unpriv;
uint32_t retval, retval_unpriv, insn_processed;
@@ -604,6 +608,28 @@ static int create_sk_storage_map(void)
return fd;
}
+static int create_event_output_map(void)
+{
+ struct bpf_create_map_attr attr = {
+ .name = "test_map",
+ .map_type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
+ .key_size = 4,
+ .value_size = 4,
+ .max_entries = 1,
+ };
+ int fd, btf_fd;
+
+ btf_fd = load_btf();
+ if (btf_fd < 0)
+ return -1;
+ attr.btf_fd = btf_fd;
+ fd = bpf_create_map_xattr(&attr);
+ close(attr.btf_fd);
+ if (fd < 0)
+ printf("Failed to create event_output\n");
+ return fd;
+}
+
static char bpf_vlog[UINT_MAX >> 8];
static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
@@ -627,6 +653,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
int *fixup_map_array_wo = test->fixup_map_array_wo;
int *fixup_map_array_small = test->fixup_map_array_small;
int *fixup_sk_storage_map = test->fixup_sk_storage_map;
+ int *fixup_map_event_output = test->fixup_map_event_output;
if (test->fill_helper) {
test->fill_insns = calloc(MAX_TEST_INSNS, sizeof(struct bpf_insn));
@@ -788,6 +815,13 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
fixup_sk_storage_map++;
} while (*fixup_sk_storage_map);
}
+ if (*fixup_map_event_output) {
+ map_fds[18] = create_event_output_map();
+ do {
+ prog[*fixup_map_event_output].imm = map_fds[18];
+ fixup_map_event_output++;
+ } while (*fixup_map_event_output);
+ }
}
static int set_admin(bool admin)
diff --git a/tools/testing/selftests/bpf/verifier/event_output.c b/tools/testing/selftests/bpf/verifier/event_output.c
new file mode 100644
index 000000000000..b25eabcfaa56
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/event_output.c
@@ -0,0 +1,94 @@
+/* instructions used to output a skb based software event, produced
+ * from code snippet:
+struct TMP {
+ uint64_t tmp;
+} tt;
+tt.tmp = 5;
+bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
+ &tt, sizeof(tt));
+return 1;
+
+the bpf assembly from llvm is:
+ 0: b7 02 00 00 05 00 00 00 r2 = 5
+ 1: 7b 2a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r2
+ 2: bf a4 00 00 00 00 00 00 r4 = r10
+ 3: 07 04 00 00 f8 ff ff ff r4 += -8
+ 4: 18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0ll
+ 6: b7 03 00 00 00 00 00 00 r3 = 0
+ 7: b7 05 00 00 08 00 00 00 r5 = 8
+ 8: 85 00 00 00 19 00 00 00 call 25
+ 9: b7 00 00 00 01 00 00 00 r0 = 1
+ 10: 95 00 00 00 00 00 00 00 exit
+
+ The reason I put the code here instead of fill_helpers is that map fixup is
+ against the insns, instead of filled prog.
+*/
+
+#define __PERF_EVENT_INSNS__ \
+ BPF_MOV64_IMM(BPF_REG_2, 5), \
+ BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -8), \
+ BPF_MOV64_REG(BPF_REG_4, BPF_REG_10), \
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -8), \
+ BPF_LD_MAP_FD(BPF_REG_2, 0), \
+ BPF_MOV64_IMM(BPF_REG_3, 0), \
+ BPF_MOV64_IMM(BPF_REG_5, 8), \
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, \
+ BPF_FUNC_perf_event_output), \
+ BPF_MOV64_IMM(BPF_REG_0, 1), \
+ BPF_EXIT_INSN(),
+{
+ "perfevent for sockops",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SOCK_OPS,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for tc",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for lwt out",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_LWT_OUT,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for xdp",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_XDP,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for socket filter",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SOCKET_FILTER,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for sk_skb",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SK_SKB,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for cgroup skb",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH] bpf: Allow bpf_skb_event_output for a few prog types
From: allanzhang @ 2019-06-24 21:58 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, David S. Miller, netdev, bpf, linux-kernel
Cc: allanzhang
Software event output is only enabled by a few prog types right now (TC,
LWT out, XDP, sockops). Many other skb based prog types need
bpf_skb_event_output to produce software event.
Added socket_filter, cg_skb, sk_skb prog types to generate sw event.
Test bpf code is generated from code snippet:
struct TMP {
uint64_t tmp;
} tt;
tt.tmp = 5;
bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
&tt, sizeof(tt));
return 1;
the bpf assembly from llvm is:
0: b7 02 00 00 05 00 00 00 r2 = 5
1: 7b 2a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r2
2: bf a4 00 00 00 00 00 00 r4 = r10
3: 07 04 00 00 f8 ff ff ff r4 += -8
4: 18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0ll
6: b7 03 00 00 00 00 00 00 r3 = 0
7: b7 05 00 00 08 00 00 00 r5 = 8
8: 85 00 00 00 19 00 00 00 call 25
9: b7 00 00 00 01 00 00 00 r0 = 1
10: 95 00 00 00 00 00 00 00 exit
Patch 1 is enabling code.
Patch 2 is fullly covered selftest code.
Signed-off-by: allanzhang <allanzhang@google.com>
^ permalink raw reply
* Re: [PATCH mlx5-next] net/mlx5: Convert mkey_table to XArray
From: Saeed Mahameed @ 2019-06-24 22:05 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Jason Gunthorpe, willy@infradead.org, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org
In-Reply-To: <20190620070305.31632-1-saeedm@mellanox.com>
On Thu, 2019-06-20 at 07:03 +0000, Saeed Mahameed wrote:
> From: Matthew Wilcox <willy@infradead.org>
>
> The lock protecting the data structure does not need to be an
> rwlock. The
> only read access to the lock is in an error path, and if that's
> limiting
> your scalability, you have bigger performance problems.
>
> Eliminate mlx5_mkey_table in favour of using the xarray directly.
> reg_mr_callback must use GFP_ATOMIC for allocating XArray nodes as it
> may
> be called in interrupt context.
>
> This also fixes a minor bug where SRCU locking was being used on the
> radix
> tree read side, when RCU was needed too.
>
> Change-Id: Ifeedbbc8f2c856cc044094d03167aae9f9162400
> Signed-off-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
>
Applied to mlx5-next.
Thanks,
Saeed.
^ permalink raw reply
* Re: [PATCH net-next v2 1/8] net: aquantia: replace internal driver version code with uts
From: Andrew Lunn @ 2019-06-24 22:08 UTC (permalink / raw)
To: Igor Russkikh; +Cc: David S . Miller, netdev@vger.kernel.org
In-Reply-To: <bb06ad821aeb27c31d1370fe7ca4ebdf73d45a06.1561388549.git.igor.russkikh@aquantia.com>
On Mon, Jun 24, 2019 at 03:10:45PM +0000, Igor Russkikh wrote:
> As it was discussed some time previously, driver is better to
> report kernel version string, as it in a best way identifies
> the codebase.
>
> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Hi Igor
You should add any reviewed-by, or acked-by tags you received for
previous versions.
^ permalink raw reply
* Re: [PATCH V3 05/10] net: dsa: microchip: Use PORT_CTRL_ADDR() instead of indirect function call
From: Marek Vasut @ 2019-06-24 22:12 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <20190624032036.GL28942@lunn.ch>
On 6/24/19 5:20 AM, Andrew Lunn wrote:
> On Mon, Jun 24, 2019 at 12:35:03AM +0200, Marek Vasut wrote:
>> The indirect function call to dev->dev_ops->get_port_addr() is expensive
>> especially if called for every single register access, and only returns
>> the value of PORT_CTRL_ADDR() macro. Use PORT_CTRL_ADDR() macro directly
>> instead.
>
> Hi Marek
>
> Rather than change just one instance, it would be better to change
> them all. And then remove dev_ops->get_port_addr().
So that actually doesn't work. The rest of the calls are in common code
(ksz_common.h) and I plan to add the KSZ8795, which has different
spacing between the ports, so those have to stay.
Although, depending on how things look, I will do more regmap cleanups,
the driver needs it. Since I have the KSZ9477 devkit, that also makes it
much easier to test the changes. I think inlining those custom accessors
would be high on the list, because they are just a mess. But that's for
another series.
--
Best regards,
Marek Vasut
^ permalink raw reply
* Re: [PATCH V3 07/10] net: dsa: microchip: Initial SPI regmap support
From: Marek Vasut @ 2019-06-24 22:03 UTC (permalink / raw)
To: netdev; +Cc: Andrew Lunn, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <20190623223508.2713-8-marex@denx.de>
On 6/24/19 12:35 AM, Marek Vasut wrote:
> Add basic SPI regmap support into the driver.
>
> Previous patches unconver that ksz_spi_write() is always ever called
> with len = 1, 2 or 4. We can thus drop the if (len > SPI_TX_BUF_LEN)
> check and we can also drop the allocation of the txbuf which is part
> of the driver data and wastes 256 bytes for no reason. Regmap covers
> the whole thing now.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Tristram Ha <Tristram.Ha@microchip.com>
> Cc: Woojung Huh <Woojung.Huh@microchip.com>
[...]
> +#define KS_SPIOP_FLAG_MASK(opcode) \
> + cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
So the robot is complaining about this. I believe this is correct, as
the mask should be in native endianness on the register and NOT the
native endianness of the CPU.
I think a cast would help here, e.g.:
- cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
- (__force unsigned long)cpu_to_be32((opcode) << (SPI_ADDR_SHIFT +
SPI_TURNAROUND_SHIFT))
Does this make sense ?
> +#define KSZ_REGMAP_COMMON(width) \
> + { \
> + .val_bits = (width), \
> + .reg_stride = (width) / 8, \
> + .reg_bits = SPI_ADDR_SHIFT + SPI_ADDR_ALIGN, \
> + .pad_bits = SPI_TURNAROUND_SHIFT, \
> + .max_register = BIT(SPI_ADDR_SHIFT) - 1, \
> + .cache_type = REGCACHE_NONE, \
> + .read_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_RD), \
> + .write_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_WR), \
[...]
--
Best regards,
Marek Vasut
^ permalink raw reply
* [PATCH net-next 0/2] Track recursive calls in TC act_mirred
From: John Hurley @ 2019-06-24 22:13 UTC (permalink / raw)
To: netdev
Cc: davem, fw, jhs, simon.horman, jakub.kicinski, oss-drivers,
John Hurley
These patches aim to prevent act_mirred causing stack overflow events from
recursively calling packet xmit or receive functions. Such events can
occur with poor TC configuration that causes packets to travel in loops
within the system.
Florian Westphal advises that a recursion crash and packets looping are
separate issues and should be treated as such. David Miller futher points
out that pcpu counters cannot track the precise skb context required to
detect loops. Hence these patches are not aimed at detecting packet loops,
rather, preventing stack flows arising from such loops.
John Hurley (2):
net: sched: refactor reinsert action
net: sched: protect against stack overflow in TC act_mirred
include/net/pkt_cls.h | 2 +-
include/net/sch_generic.h | 2 +-
net/core/dev.c | 4 +---
net/sched/act_mirred.c | 17 ++++++++++++++++-
4 files changed, 19 insertions(+), 6 deletions(-)
--
2.7.4
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox