* [PATCH net-next v3 6/6] devlink: add missing NULL checks for devlink ops
From: Jakub Kicinski @ 2019-02-22 22:07 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190222220758.7117-1-jakub.kicinski@netronome.com>
Commit 76726ccb7f46 ("devlink: add flash update command") and
commit 2d8dc5bbf4e7 ("devlink: Add support for reload")
access devlink ops without NULL-checking. Add the missing checks.
Note that all drivers currently implementing devlink pass non-NULL
ops, so this is not a problem.
Reported-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
net/core/devlink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 78c6ea1870ca..38cb0239dede 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2651,7 +2651,7 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
struct devlink *devlink = info->user_ptr[0];
int err;
- if (!devlink->ops->reload)
+ if (!devlink->ops || !devlink->ops->reload)
return -EOPNOTSUPP;
err = devlink_resources_validate(devlink, NULL, info);
@@ -2669,7 +2669,7 @@ static int devlink_nl_cmd_flash_update(struct sk_buff *skb,
const char *file_name, *component;
struct nlattr *nla_component;
- if (!devlink->ops->flash_update)
+ if (!devlink->ops || !devlink->ops->flash_update)
return -EOPNOTSUPP;
if (!info->attrs[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME])
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v3 4/6] nfp: remove ethtool flashing fallback
From: Jakub Kicinski @ 2019-02-22 22:07 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190222220758.7117-1-jakub.kicinski@netronome.com>
Now that devlink fallback will be called reliably, we can remove
the ethtool flashing code.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../ethernet/netronome/nfp/nfp_net_ethtool.c | 24 -------------------
1 file changed, 24 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 8f189149efc5..690b62718dbb 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -1234,28 +1234,6 @@ static int nfp_net_set_channels(struct net_device *netdev,
return nfp_net_set_num_rings(nn, total_rx, total_tx);
}
-static int
-nfp_net_flash_device(struct net_device *netdev, struct ethtool_flash *flash)
-{
- struct nfp_app *app;
- int ret;
-
- if (flash->region != ETHTOOL_FLASH_ALL_REGIONS)
- return -EOPNOTSUPP;
-
- app = nfp_app_from_netdev(netdev);
- if (!app)
- return -EOPNOTSUPP;
-
- dev_hold(netdev);
- rtnl_unlock();
- ret = nfp_flash_update_common(app->pf, flash->data, NULL);
- rtnl_lock();
- dev_put(netdev);
-
- return ret;
-}
-
static const struct ethtool_ops nfp_net_ethtool_ops = {
.get_drvinfo = nfp_net_get_drvinfo,
.get_link = ethtool_op_get_link,
@@ -1266,7 +1244,6 @@ static const struct ethtool_ops nfp_net_ethtool_ops = {
.get_sset_count = nfp_net_get_sset_count,
.get_rxnfc = nfp_net_get_rxnfc,
.set_rxnfc = nfp_net_set_rxnfc,
- .flash_device = nfp_net_flash_device,
.get_rxfh_indir_size = nfp_net_get_rxfh_indir_size,
.get_rxfh_key_size = nfp_net_get_rxfh_key_size,
.get_rxfh = nfp_net_get_rxfh,
@@ -1292,7 +1269,6 @@ const struct ethtool_ops nfp_port_ethtool_ops = {
.get_strings = nfp_port_get_strings,
.get_ethtool_stats = nfp_port_get_stats,
.get_sset_count = nfp_port_get_sset_count,
- .flash_device = nfp_net_flash_device,
.set_dump = nfp_app_set_dump,
.get_dump_flag = nfp_app_get_dump_flag,
.get_dump_data = nfp_app_get_dump_data,
--
2.19.2
^ permalink raw reply related
* Re: [PATCH net-next v2] ip_tunnel: Add dst_cache management lwtunnel_state of ip tunnel
From: David Ahern @ 2019-02-22 22:16 UTC (permalink / raw)
To: wenxu, davem, netdev
In-Reply-To: <8b96ffcb-fe29-1c49-a266-3e9d20b60d89@ucloud.cn>
On 2/22/19 10:21 AM, wenxu wrote:
> On 2019/2/22 下午11:03, David Ahern wrote:
>> On 2/21/19 11:14 PM, wenxu wrote:
>>> build_state in the rcu_read_lock and disable the preempt
>>>
>>> rcu_read_lock();
>>> ops = rcu_dereference(lwtun_encaps[encap_type]);
>>> if (likely(ops && ops->build_state && try_module_get(ops->owner))) {
>>> found = true;
>>> ret = ops->build_state(encap, family, cfg, lws, extack);
>>> if (ret)
>>> module_put(ops->owner);
>>> }
>>> rcu_read_unlock();
>>>
>> Missed that.
>>
>> Once a reference is taken the rcu_read_lock can be dropped before
>> calling build_state allowing the allocations to be GFP_KERNEL.
>>
> I don't think the rcu_read_lock can be dropped. The whole operation of the reference should be protect
>
> in the rcu_read_lock that ensure the ops will not be destroy (if it will be)
>
If 'try_module_get(ops->owner)' succeeds then a reference is held so the
ops will not go away.
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Jann Horn @ 2019-02-22 22:17 UTC (permalink / raw)
To: Nadav Amit
Cc: Andy Lutomirski, Alexei Starovoitov, Steven Rostedt,
Linus Torvalds, Masami Hiramatsu, Linux List Kernel Mailing,
Ingo Molnar, Andrew Morton, Changbin Du, Kees Cook,
Andy Lutomirski, Daniel Borkmann, Network Development,
bpf@vger.kernel.org, Rick Edgecombe, Dave Hansen,
Peter Zijlstra (Intel)
In-Reply-To: <0ED6836E-3432-4E1C-BABC-BEA6BDD36287@vmware.com>
On Fri, Feb 22, 2019 at 11:08 PM Nadav Amit <namit@vmware.com> wrote:
> > On Feb 22, 2019, at 1:43 PM, Jann Horn <jannh@google.com> wrote:
> >
> > (adding some people from the text_poke series to the thread, removing stable@)
> >
> > On Fri, Feb 22, 2019 at 8:55 PM Andy Lutomirski <luto@amacapital.net> wrote:
> >>> On Feb 22, 2019, at 11:34 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >>>> On Fri, Feb 22, 2019 at 02:30:26PM -0500, Steven Rostedt wrote:
> >>>> On Fri, 22 Feb 2019 11:27:05 -0800
> >>>> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >>>>
> >>>>>> On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
> >>>>>>
> >>>>>> Then we should still probably fix up "__probe_kernel_read()" to not
> >>>>>> allow user accesses. The easiest way to do that is actually likely to
> >>>>>> use the "unsafe_get_user()" functions *without* doing a
> >>>>>> uaccess_begin(), which will mean that modern CPU's will simply fault
> >>>>>> on a kernel access to user space.
> >>>>>
> >>>>> On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
> >>>>> and users pass both user and kernel addresses into it and expect
> >>>>> that the helper will actually try to read from that address.
> >>>>>
> >>>>> If __probe_kernel_read will suddenly start failing on all user addresses
> >>>>> it will break the expectations.
> >>>>> How do we solve it in bpf_probe_read?
> >>>>> Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
> >>>>> in the loop?
> >>>>> That's doable, but people already complain that bpf_probe_read() is slow
> >>>>> and shows up in their perf report.
> >>>>
> >>>> We're changing kprobes to add a specific flag to say that we want to
> >>>> differentiate between kernel or user reads. Can this be done with
> >>>> bpf_probe_read()? If it's showing up in perf report, I doubt a single
> >>>
> >>> so you're saying you will break existing kprobe scripts?
> >>> I don't think it's a good idea.
> >>> It's not acceptable to break bpf_probe_read uapi.
> >>
> >> If so, the uapi is wrong: a long-sized number does not reliably identify an address if you don’t separately know whether it’s a user or kernel address. s390x and 4G:4G x86_32 are the notable exceptions. I have lobbied for RISC-V and future x86_64 to join the crowd. I don’t know whether I’ll win this fight, but the uapi will probably have to change for at least s390x.
> >>
> >> What to do about existing scripts is a different question.
> >
> > This lack of logical separation between user and kernel addresses
> > might interact interestingly with the text_poke series, specifically
> > "[PATCH v3 05/20] x86/alternative: Initialize temporary mm for
> > patching" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-6-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cd44d6f0765dd49b20db708d6990ee7e8%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864686717142892&sdata=gVALdkEULEhj4iJNEWAGxyYWe2lxnHRdamW5ZA2A5RQ%3D&reserved=0)
> > and "[PATCH v3 06/20] x86/alternative: Use temporary mm for text
> > poking" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-7-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cd44d6f0765dd49b20db708d6990ee7e8%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864686717142892&sdata=nu2J1FtJsZJmt53SKJz8C8ktWE9eycwdAA%2BiCi1TfCc%3D&reserved=0),
> > right? If someone manages to get a tracing BPF program to trigger in a
> > task that has switched to the patching mm, could they use
> > bpf_probe_write_user() - which uses probe_kernel_write() after
> > checking that KERNEL_DS isn't active and that access_ok() passes - to
> > overwrite kernel text that is mapped writable in the patching mm?
>
> Yes, this is a good point. I guess text_poke() should be defined with
> “__kprobes” and open-code memcpy.
>
> Does it sound reasonable?
Doesn't __text_poke() as implemented in the proposed patch use a
couple other kernel functions, too? Like switch_mm_irqs_off() and
pte_clear() (which can be a call into a separate function on paravirt
kernels)?
^ permalink raw reply
* Re: [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup
From: Stefano Brivio @ 2019-02-22 22:22 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern
In-Reply-To: <06a88ed3b2efab312f6301e9fe8eabd4383a9d3a.1550851038.git.pabeni@redhat.com>
On Fri, 22 Feb 2019 17:06:33 +0100
Paolo Abeni <pabeni@redhat.com> wrote:
> @@ -135,7 +135,9 @@ tests="
> pmtu_vti6_default_mtu vti6: default MTU assignment
> pmtu_vti4_link_add_mtu vti4: MTU setting on link creation
> pmtu_vti6_link_add_mtu vti6: MTU setting on link creation
> - pmtu_vti6_link_change_mtu vti6: MTU changes on link changes"
> + pmtu_vti6_link_change_mtu vti6: MTU changes on link changes
> + pmtu_ipv4_exception_cleanup ipv4: cleanup of cached exceptions
> + pmtu_ipv6_exception_cleanup ipv6: cleanup of cached exceptions"
I forgot: while at it, could you please also add test descriptions above
as it was done for the other tests? Thank you.
--
Stefano
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Nadav Amit @ 2019-02-22 22:21 UTC (permalink / raw)
To: Jann Horn
Cc: Andy Lutomirski, Alexei Starovoitov, Steven Rostedt,
Linus Torvalds, Masami Hiramatsu, Linux List Kernel Mailing,
Ingo Molnar, Andrew Morton, Changbin Du, Kees Cook,
Andy Lutomirski, Daniel Borkmann, Network Development,
bpf@vger.kernel.org, Rick Edgecombe, Dave Hansen,
Peter Zijlstra (Intel)
In-Reply-To: <CAG48ez10RH2YhQ0wnEDOpKoGjrociq+zK0En6Aq06Yg9W3o5aQ@mail.gmail.com>
> On Feb 22, 2019, at 2:17 PM, Jann Horn <jannh@google.com> wrote:
>
> On Fri, Feb 22, 2019 at 11:08 PM Nadav Amit <namit@vmware.com> wrote:
>>> On Feb 22, 2019, at 1:43 PM, Jann Horn <jannh@google.com> wrote:
>>>
>>> (adding some people from the text_poke series to the thread, removing stable@)
>>>
>>> On Fri, Feb 22, 2019 at 8:55 PM Andy Lutomirski <luto@amacapital.net> wrote:
>>>>> On Feb 22, 2019, at 11:34 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>>>>>> On Fri, Feb 22, 2019 at 02:30:26PM -0500, Steven Rostedt wrote:
>>>>>> On Fri, 22 Feb 2019 11:27:05 -0800
>>>>>> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>>>>>>
>>>>>>>> On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
>>>>>>>>
>>>>>>>> Then we should still probably fix up "__probe_kernel_read()" to not
>>>>>>>> allow user accesses. The easiest way to do that is actually likely to
>>>>>>>> use the "unsafe_get_user()" functions *without* doing a
>>>>>>>> uaccess_begin(), which will mean that modern CPU's will simply fault
>>>>>>>> on a kernel access to user space.
>>>>>>>
>>>>>>> On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
>>>>>>> and users pass both user and kernel addresses into it and expect
>>>>>>> that the helper will actually try to read from that address.
>>>>>>>
>>>>>>> If __probe_kernel_read will suddenly start failing on all user addresses
>>>>>>> it will break the expectations.
>>>>>>> How do we solve it in bpf_probe_read?
>>>>>>> Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
>>>>>>> in the loop?
>>>>>>> That's doable, but people already complain that bpf_probe_read() is slow
>>>>>>> and shows up in their perf report.
>>>>>>
>>>>>> We're changing kprobes to add a specific flag to say that we want to
>>>>>> differentiate between kernel or user reads. Can this be done with
>>>>>> bpf_probe_read()? If it's showing up in perf report, I doubt a single
>>>>>
>>>>> so you're saying you will break existing kprobe scripts?
>>>>> I don't think it's a good idea.
>>>>> It's not acceptable to break bpf_probe_read uapi.
>>>>
>>>> If so, the uapi is wrong: a long-sized number does not reliably identify an address if you don’t separately know whether it’s a user or kernel address. s390x and 4G:4G x86_32 are the notable exceptions. I have lobbied for RISC-V and future x86_64 to join the crowd. I don’t know whether I’ll win this fight, but the uapi will probably have to change for at least s390x.
>>>>
>>>> What to do about existing scripts is a different question.
>>>
>>> This lack of logical separation between user and kernel addresses
>>> might interact interestingly with the text_poke series, specifically
>>> "[PATCH v3 05/20] x86/alternative: Initialize temporary mm for
>>> patching" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-6-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cf2513009ef734ecd6b0d08d69913a5ae%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864707020821793&sdata=HAbnDcrBne64JyPuVUMKmM7nQk67F%2BFvjuXEn8TmHeo%3D&reserved=0)
>>> and "[PATCH v3 06/20] x86/alternative: Use temporary mm for text
>>> poking" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-7-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cf2513009ef734ecd6b0d08d69913a5ae%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864707020821793&sdata=vNRIMKtFDy%2F3z5FlTwDiJY6VGEV%2FMHgQPTdFSFtCo4s%3D&reserved=0),
>>> right? If someone manages to get a tracing BPF program to trigger in a
>>> task that has switched to the patching mm, could they use
>>> bpf_probe_write_user() - which uses probe_kernel_write() after
>>> checking that KERNEL_DS isn't active and that access_ok() passes - to
>>> overwrite kernel text that is mapped writable in the patching mm?
>>
>> Yes, this is a good point. I guess text_poke() should be defined with
>> “__kprobes” and open-code memcpy.
>>
>> Does it sound reasonable?
>
> Doesn't __text_poke() as implemented in the proposed patch use a
> couple other kernel functions, too? Like switch_mm_irqs_off() and
> pte_clear() (which can be a call into a separate function on paravirt
> kernels)?
I will move the pte_clear() to be done after the poking mm was unloaded.
Give me a few minutes to send a sketch of what I think should be done.
^ permalink raw reply
* Re: Yet another approach for implementing connection tracking offload
From: Marcelo Leitner @ 2019-02-22 22:23 UTC (permalink / raw)
To: Yossi Kuperman
Cc: Guy Shattah, Aaron Conole, John Hurley, Simon Horman,
Justin Pettit, Gregory Rose, Eelco Chaudron, Flavio Leitner,
Florian Westphal, Jiri Pirko, Rashid Khan, Sushil Kulkarni,
Andy Gospodarek, Roi Dayan, Or Gerlitz, Rony Efraim,
davem@davemloft.net, Paul Blakey, netdev@vger.kernel.org
In-Reply-To: <65CD6CF6-C50F-47F7-85DB-12D480FA4712@mellanox.com>
On Mon, Feb 18, 2019 at 07:00:19PM +0000, Yossi Kuperman wrote:
> Hello All,
>
> Following is a description of yet another possible approach to
> implement connection tracking offload. We would like to hear your
> opinion. There is the “native” way of implementing such an offload
> by mirroring the software tables to hardware. This way seems
> straightforward and simple, but real life is much more complicated
> than that. Alternatively, we can merge the data-path flows
> (separated by recirc_id) and offload a single flow to hardware.
>
> The general idea is quite simple. When OVS-daemon configures TC with
> a filter that recirculate, the driver merely pretends to offload it
> and return success. Upon packet arrival (in software) we let it
This has potential to be a support nightmare: things that should and
seems to be but are actually not and expected not to be.
IOW, not_in_hw should still be there somehow.
> traverse TC as usual, except for now we notify the driver on each
> successful match. By doing this, the driver has all the necessary
> information to merge the participating flows---including connection
> tracking 5-tuple---into one equivalent flow. We do such a merge and
> offload only if the connection is established. Note: the same
> mechanism to communicate a 5-tuple to the driver can be used to
> notify on a filter match.
>
> It is the driver responsibility to build and maintain the list of
> filters a (specific) packet hit along the TC walk. Once we reach the
I'm assuming this could be a shared code amongst the drivers, like
DIM. We really don't want different algorithms for this.
With that, we would be using the driver as just a temporary storage,
for the matches/actions. Maybe we can do that with skb extensions?
Like, turn on a flight recorder extention and have one especial last
tc action (or even embed it into core tc) to process it when the
traversing finishes. And only then call the driver to update whatever
is needed.
> last filter (a terminating one, e.g., forward) the driver posts a
> work on a dedicated work-queue. In this work-queue context, we merge
> the participating filters and create a new filter that is logically
> equal (match + actions). The merge itself is not complicated as it
> might seems—TC does all the heavy lifting, this is not a random list
> of filters. At this point, we configure the hardware with one
> filter, either we have a match and the packet is handled by the
> hardware, or we don’t and the packet goes to software unmodified.
>
> Going along this path we must tackle two things: 1) counters and 2)
> TC filter deletion. 1) We must maintain TC counters as the user
> expect. Each merged filter holds a list of filters it is derived
> from, parents. Once an update is available for a merged filter
> counter, the driver must update the corresponding parents
> appropriately. 2) Upon TC filer deletion it is mandatory to remove
> all the derived (merged) filters from the hardware as consequence.
I'm failing to see how this would address two features of CT:
- window validation. How would the card know that it should perform
window validation on these packets?
- flow stats. If it offloads the result of a merge, how can it
retrieve the specific stats for each 5-tuple? Even with the lists
you mentioned, once the 5-tuples are aggregated, we can't separate
them anymore.
>
>
> Pros & Cons
>
> Pros: 1) Circumvent the complexity involved with continuation in
> software where the hardware left off. 2) Simplifies the hardware
> pipeline with only one filter and might improve the overall
> performance.
>
> Cons: 1) Only applicable to OVS-oriented filters, will not support
> priorities and overlapping filters. 2) Merger logic might consume
> CPU cycles which might impact the rate of filters we can offload.
> However, this overhead is believed to be negligible, if implemented
> carefully. 3) Requires TC/flower to notify the driver on each filter
> match (that is the only change needed above the driver).
On more general comments, I have the feeling that this would be best
done in OvS:
- it has 2 flow "translations": one from OpenFlow to OvS/tc sw
datapath, and then OvS/tc sw to hw. If the resulting flows are
better for the hw, aren't them better for sw datapath too?
- if this is for OvS-oriented filters, why can't/shouldn't OvS do this
processing instead?
Somehow I keep thinking this is replacing conntrack (as in the
conntrack subsystem) with ipset offloading.
All in all, any approach is probably fine but I'm struggling to see a
path forward from this to actual CT offloading. Maybe I missed
something.
Cheers,
Marcelo
>
>
> Both approaches share the same software model, most of the code
> above the driver is shared. This approach can be considered
> temporary until the hardware will mature.
>
> What do you think about this approach?
>
> If something is not clear please let me know and I will do my best
> to clarify.
>
> Cheers, Kuperman
>
^ permalink raw reply
* Re: [PATCH][net-next] ipv6: sanitize RCU usage on fib6_next
From: David Ahern @ 2019-02-22 22:35 UTC (permalink / raw)
To: Li RongQing, netdev
In-Reply-To: <1550828682-10608-1-git-send-email-lirongqing@baidu.com>
On 2/22/19 4:44 AM, Li RongQing wrote:
> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index 6613d8dbb0e5..b73d40d68178 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -1143,7 +1143,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
>
> atomic_inc(&rt->fib6_ref);
> rcu_assign_pointer(rt->fib6_node, fn);
> - rt->fib6_next = iter->fib6_next;
> + rcu_assign_pointer(rt->fib6_next, iter->fib6_next);
> rcu_assign_pointer(*ins, rt);
> if (!info->skip_notify)
> inet6_rt_notify(RTM_NEWROUTE, rt, info, NLM_F_REPLACE);
> @@ -1761,7 +1761,9 @@ static void fib6_del_route(struct fib6_table *table, struct fib6_node *fn,
> RT6_TRACE("fib6_del_route\n");
>
> /* Unlink it */
> - *rtp = rt->fib6_next;
> + *rtp = rcu_dereference_protected(rt->fib6_next,
> + lockdep_is_held(&rt->fib6_table->tb6_lock));
> +
> rt->fib6_node = NULL;
> net->ipv6.rt6_stats->fib_rt_entries--;
> net->ipv6.rt6_stats->fib_discarded_routes++;
>
There are a number of fib6_next references not using rcu api's. Why are
you only touching these 2?
^ permalink raw reply
* [PATCH bpf 0/2] nfp: bpf: fix two ALU32 code-gen bugs
From: Jiong Wang @ 2019-02-22 22:36 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, oss-drivers, Jiong Wang
code-gen for BPF_ALU | BPF_XOR | BPF_K is wrong when imm is -1, also high
32-bit of 64-bit register should always be cleared.
This set fixed both bugs.
Jiong Wang (2):
nfp: bpf: fix code-gen bug on BPF_ALU | BPF_XOR | BPF_K
nfp: bpf: fix ALU32 high bits clearance bug
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH bpf 1/2] nfp: bpf: fix code-gen bug on BPF_ALU | BPF_XOR | BPF_K
From: Jiong Wang @ 2019-02-22 22:36 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, oss-drivers, Jiong Wang
In-Reply-To: <1550874964-16769-1-git-send-email-jiong.wang@netronome.com>
The intended optimization should be A ^ 0 = A, not A ^ -1 = A.
Fixes: cd7df56ed3e6 ("nfp: add BPF to NFP code translator")
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index e23ca90..a096965 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -2309,7 +2309,7 @@ static int xor_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
static int xor_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
- return wrp_alu32_imm(nfp_prog, meta, ALU_OP_XOR, !~meta->insn.imm);
+ return wrp_alu32_imm(nfp_prog, meta, ALU_OP_XOR, !meta->insn.imm);
}
static int and_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
--
2.7.4
^ permalink raw reply related
* [PATCH bpf 2/2] nfp: bpf: fix ALU32 high bits clearance bug
From: Jiong Wang @ 2019-02-22 22:36 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, oss-drivers, Jiong Wang
In-Reply-To: <1550874964-16769-1-git-send-email-jiong.wang@netronome.com>
NFP BPF JIT compiler is doing a couple of small optimizations when jitting
ALU imm instructions, some of these optimizations could save code-gen, for
example:
A & -1 = A
A | 0 = A
A ^ 0 = A
However, for ALU32, high 32-bit of the 64-bit register should still be
cleared according to ISA semantics.
Fixes: cd7df56ed3e6 ("nfp: add BPF to NFP code translator")
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index a096965..0a868c8 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -1291,15 +1291,10 @@ wrp_alu64_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
static int
wrp_alu32_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
- enum alu_op alu_op, bool skip)
+ enum alu_op alu_op)
{
const struct bpf_insn *insn = &meta->insn;
- if (skip) {
- meta->skip = true;
- return 0;
- }
-
wrp_alu_imm(nfp_prog, insn->dst_reg * 2, alu_op, insn->imm);
wrp_immed(nfp_prog, reg_both(insn->dst_reg * 2 + 1), 0);
@@ -2309,7 +2304,7 @@ static int xor_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
static int xor_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
- return wrp_alu32_imm(nfp_prog, meta, ALU_OP_XOR, !meta->insn.imm);
+ return wrp_alu32_imm(nfp_prog, meta, ALU_OP_XOR);
}
static int and_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
@@ -2319,7 +2314,7 @@ static int and_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
static int and_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
- return wrp_alu32_imm(nfp_prog, meta, ALU_OP_AND, !~meta->insn.imm);
+ return wrp_alu32_imm(nfp_prog, meta, ALU_OP_AND);
}
static int or_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
@@ -2329,7 +2324,7 @@ static int or_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
static int or_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
- return wrp_alu32_imm(nfp_prog, meta, ALU_OP_OR, !meta->insn.imm);
+ return wrp_alu32_imm(nfp_prog, meta, ALU_OP_OR);
}
static int add_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
@@ -2339,7 +2334,7 @@ static int add_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
static int add_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
- return wrp_alu32_imm(nfp_prog, meta, ALU_OP_ADD, !meta->insn.imm);
+ return wrp_alu32_imm(nfp_prog, meta, ALU_OP_ADD);
}
static int sub_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
@@ -2349,7 +2344,7 @@ static int sub_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
static int sub_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
- return wrp_alu32_imm(nfp_prog, meta, ALU_OP_SUB, !meta->insn.imm);
+ return wrp_alu32_imm(nfp_prog, meta, ALU_OP_SUB);
}
static int mul_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] selftests: fib_tests: sleep after changing carrier. again.
From: David Ahern @ 2019-02-22 22:37 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo, netdev; +Cc: linux-kselftest, David Ahern
In-Reply-To: <20190222102741.31173-1-cascardo@canonical.com>
On 2/22/19 5:27 AM, Thadeu Lima de Souza Cascardo wrote:
> Just like commit e2ba732a1681 ("selftests: fib_tests: sleep after
> changing carrier"), wait one second to allow linkwatch to propagate the
> carrier change to the stack.
>
> There are two sets of carrier tests. The first slept after the carrier
> was set to off, and when the second set ran, it was likely that the
> linkwatch would be able to run again without much delay, reducing the
> likelihood of a race. However, if you run 'fib_tests.sh -t carrier' on a
> loop, you will quickly notice the failures.
>
> Sleeping on the second set of tests make the failures go away.
>
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
> ---
> tools/testing/selftests/net/fib_tests.sh | 1 +
> 1 file changed, 1 insertion(+)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Nadav Amit @ 2019-02-22 22:39 UTC (permalink / raw)
To: Jann Horn
Cc: Andy Lutomirski, Alexei Starovoitov, Steven Rostedt,
Linus Torvalds, Masami Hiramatsu, Linux List Kernel Mailing,
Ingo Molnar, Andrew Morton, Changbin Du, Kees Cook,
Andy Lutomirski, Daniel Borkmann, Network Development,
bpf@vger.kernel.org, Rick Edgecombe, Dave Hansen,
Peter Zijlstra (Intel), Igor Stoppa
In-Reply-To: <7701651F-F10E-4212-925E-1CB77C5D3E69@vmware.com>
> On Feb 22, 2019, at 2:21 PM, Nadav Amit <namit@vmware.com> wrote:
>
>> On Feb 22, 2019, at 2:17 PM, Jann Horn <jannh@google.com> wrote:
>>
>> On Fri, Feb 22, 2019 at 11:08 PM Nadav Amit <namit@vmware.com> wrote:
>>>> On Feb 22, 2019, at 1:43 PM, Jann Horn <jannh@google.com> wrote:
>>>>
>>>> (adding some people from the text_poke series to the thread, removing stable@)
>>>>
>>>> On Fri, Feb 22, 2019 at 8:55 PM Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>> On Feb 22, 2019, at 11:34 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>>>>>>> On Fri, Feb 22, 2019 at 02:30:26PM -0500, Steven Rostedt wrote:
>>>>>>> On Fri, 22 Feb 2019 11:27:05 -0800
>>>>>>> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>>>>>>>
>>>>>>>>> On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
>>>>>>>>>
>>>>>>>>> Then we should still probably fix up "__probe_kernel_read()" to not
>>>>>>>>> allow user accesses. The easiest way to do that is actually likely to
>>>>>>>>> use the "unsafe_get_user()" functions *without* doing a
>>>>>>>>> uaccess_begin(), which will mean that modern CPU's will simply fault
>>>>>>>>> on a kernel access to user space.
>>>>>>>>
>>>>>>>> On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
>>>>>>>> and users pass both user and kernel addresses into it and expect
>>>>>>>> that the helper will actually try to read from that address.
>>>>>>>>
>>>>>>>> If __probe_kernel_read will suddenly start failing on all user addresses
>>>>>>>> it will break the expectations.
>>>>>>>> How do we solve it in bpf_probe_read?
>>>>>>>> Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
>>>>>>>> in the loop?
>>>>>>>> That's doable, but people already complain that bpf_probe_read() is slow
>>>>>>>> and shows up in their perf report.
>>>>>>>
>>>>>>> We're changing kprobes to add a specific flag to say that we want to
>>>>>>> differentiate between kernel or user reads. Can this be done with
>>>>>>> bpf_probe_read()? If it's showing up in perf report, I doubt a single
>>>>>>
>>>>>> so you're saying you will break existing kprobe scripts?
>>>>>> I don't think it's a good idea.
>>>>>> It's not acceptable to break bpf_probe_read uapi.
>>>>>
>>>>> If so, the uapi is wrong: a long-sized number does not reliably identify an address if you don’t separately know whether it’s a user or kernel address. s390x and 4G:4G x86_32 are the notable exceptions. I have lobbied for RISC-V and future x86_64 to join the crowd. I don’t know whether I’ll win this fight, but the uapi will probably have to change for at least s390x.
>>>>>
>>>>> What to do about existing scripts is a different question.
>>>>
>>>> This lack of logical separation between user and kernel addresses
>>>> might interact interestingly with the text_poke series, specifically
>>>> "[PATCH v3 05/20] x86/alternative: Initialize temporary mm for
>>>> patching" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-6-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cf2513009ef734ecd6b0d08d69913a5ae%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864707020821793&sdata=HAbnDcrBne64JyPuVUMKmM7nQk67F%2BFvjuXEn8TmHeo%3D&reserved=0)
>>>> and "[PATCH v3 06/20] x86/alternative: Use temporary mm for text
>>>> poking" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-7-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cf2513009ef734ecd6b0d08d69913a5ae%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864707020821793&sdata=vNRIMKtFDy%2F3z5FlTwDiJY6VGEV%2FMHgQPTdFSFtCo4s%3D&reserved=0),
>>>> right? If someone manages to get a tracing BPF program to trigger in a
>>>> task that has switched to the patching mm, could they use
>>>> bpf_probe_write_user() - which uses probe_kernel_write() after
>>>> checking that KERNEL_DS isn't active and that access_ok() passes - to
>>>> overwrite kernel text that is mapped writable in the patching mm?
>>>
>>> Yes, this is a good point. I guess text_poke() should be defined with
>>> “__kprobes” and open-code memcpy.
>>>
>>> Does it sound reasonable?
>>
>> Doesn't __text_poke() as implemented in the proposed patch use a
>> couple other kernel functions, too? Like switch_mm_irqs_off() and
>> pte_clear() (which can be a call into a separate function on paravirt
>> kernels)?
>
> I will move the pte_clear() to be done after the poking mm was unloaded.
> Give me a few minutes to send a sketch of what I think should be done.
Err.. You are right, I don’t see an easy way of preventing a kprobe from
being set on switch_mm_irqs_off(), and open-coding this monster is too ugly.
The reasonable solution seems to me as taking all the relevant pieces of
code (and data) that might be used during text-poking and encapsulating them, so they
will be set in a memory area which cannot be kprobe'd. This can also be
useful to write-protect data structures of code that calls text_poke(),
e.g., static-keys. It can also protect data on that stack that is used
during text_poke() from being overwritten from another core.
This solution is somewhat similar to Igor Stoppa’s idea of using “enclaves”
when doing write-rarely operations.
Right now, I think that text_poke() will keep being susceptible to such
an attack, unless you have a better suggestion.
^ permalink raw reply
* [PATCH net-next 0/5] net: phy: aquantia: improve and extend driver
From: Heiner Kallweit @ 2019-02-22 22:45 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
This series improves and extends the Aquantia PHY driver.
Andrew Lunn (1):
net: phy: aquantia: use genphy_c45_an_config_aneg
Heiner Kallweit (4):
net: phy: aquantia: remove false 5G and 10G speed ability for AQCS109
net: phy: don't change modes we don't care about in genphy_c45_read_lpa
net: phy: add genphy_c45_read_status
net: phy: aquantia: use genphy_c45_read_status
drivers/net/phy/aquantia.c | 94 ++++++++++++++++++++++++++------------
drivers/net/phy/phy-c45.c | 35 +++++++++++++-
include/linux/phy.h | 1 +
3 files changed, 99 insertions(+), 31 deletions(-)
--
2.20.1
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Alexei Starovoitov @ 2019-02-22 22:51 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Miller, Masami Hiramatsu, Steven Rostedt, Andy Lutomirski,
Linux List Kernel Mailing, Ingo Molnar, Andrew Morton, stable,
Changbin Du, Jann Horn, Kees Cook, Andrew Lutomirski,
Daniel Borkmann, Netdev, bpf
In-Reply-To: <CAHk-=wh9uWJaih5i6qBPer5rzcs=A+yb_ejU093b1esAhoFveQ@mail.gmail.com>
On Fri, Feb 22, 2019 at 01:59:10PM -0800, Linus Torvalds wrote:
> On Fri, Feb 22, 2019 at 1:38 PM David Miller <davem@davemloft.net> wrote:
> >
> > Don't be surprised if we see more separation like this in the future too.
>
> Yes, with the whole meltdown fiasco, there's actually more pressure to
> add more support for separation of kernel/user address spaces. As Andy
> pointed out, it's been discussed as a future wish-list for x86-64 too.
>
> But yeah, right now the *common* architectures all distinguish kernel
> and user space by pointers (ie x86-64, arm64 and powerpc).
That's all fine. I'm missing rationale for making probe_kernel_read()
fail on user addresses.
What is fundamentally wrong with a function probe_any_address_read() ?
For context, typical bpf kprobe program looks like this:
#define probe_read(P) \
({typeof(P) val = 0; bpf_probe_read(&val, sizeof(val), &P); val;})
SEC("kprobe/__set_task_comm")
int bpf_prog(struct pt_regs *ctx)
{
struct signal_struct *signal;
struct task_struct *tsk;
char oldcomm[16] = {};
char newcomm[16] = {};
u16 oom_score_adj;
u32 pid;
tsk = (void *)PT_REGS_PARM1(ctx);
pid = probe_read(tsk->pid);
bpf_probe_read(oldcomm, sizeof(oldcomm), &tsk->comm);
bpf_probe_read(newcomm, sizeof(newcomm), (void *)PT_REGS_PARM2(ctx));
signal = probe_read(tsk->signal);
oom_score_adj = probe_read(signal->oom_score_adj);
...
}
where PT_REGS_PARMx macros are defined per architecture.
On x86 it's #define PT_REGS_PARM1(x) ((x)->di)
The program writer has to know the meaning of function arguments.
In this example they need to know that __set_task_comm is defined as
void __set_task_comm(struct task_struct *tsk, const char *buf, bool exec) in the kernel.
Right now these programs just call bpf_probe_read() on whatever data
they need to access and not differentiating whether it's user or kernel.
One idea we discussed is to split bpf_probe_read() into kernel_read and user_read
helpers, but in the BPF verifier we cannot determine which address space
the program wants to access. The prog writer needs to manually analyze the program
to use correct one. But mistakes are possible and cannot be fatal.
On the kernel side we have to be safe.
Both probe_kernel_read and probe_user_read must not panic if a pointer
from wrong address space was passed.
Hence my preference is to keep probe_kernel_read as "try read any address".
The function can be renamed to indicate so.
^ permalink raw reply
* [PATCH net-next 1/5] net: phy: aquantia: remove false 5G and 10G speed ability for AQCS109
From: Heiner Kallweit @ 2019-02-22 22:48 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <4ae20155-367d-4b11-63f9-28580b7cf1cb@gmail.com>
AQCS109 belongs to a family of PHY's where certain members don't
support 5G or 10G. However for all members of the family the chip
reports 10G and 5G capability. Therefore remove the not supported
modes for AQCS109.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/aquantia.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index 34be44d00..9661ef4b4 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -152,6 +152,15 @@ static int aqr_read_status(struct phy_device *phydev)
return 0;
}
+static int aqcs109_config_init(struct phy_device *phydev)
+{
+ /* AQCS109 belongs to a chip family partially supporting 10G and 5G.
+ * PMA speed ability bits are the same for all members of the family,
+ * AQCS109 however supports speeds up to 2.5G only.
+ */
+ return phy_set_max_speed(phydev, SPEED_2500);
+}
+
static struct phy_driver aqr_driver[] = {
{
PHY_ID_MATCH_MODEL(PHY_ID_AQ1202),
@@ -208,6 +217,7 @@ static struct phy_driver aqr_driver[] = {
.name = "Aquantia AQCS109",
.aneg_done = genphy_c45_aneg_done,
.get_features = genphy_c45_pma_read_abilities,
+ .config_init = aqcs109_config_init,
.config_aneg = aqr_config_aneg,
.config_intr = aqr_config_intr,
.ack_interrupt = aqr_ack_interrupt,
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 2/5] net: phy: aquantia: add support for auto-negotiation configuration
From: Heiner Kallweit @ 2019-02-22 22:49 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <4ae20155-367d-4b11-63f9-28580b7cf1cb@gmail.com>
From: Andrew Lunn <andrew@lunn.ch>
Make use of the generic c45 code, plus code specific to the Aquantia
phy for 1000BaseT negotiation.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/aquantia.c | 40 +++++++++++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index 9661ef4b4..a1846daa3 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -20,6 +20,10 @@
#define PHY_ID_AQCS109 0x03a1b5c2
#define PHY_ID_AQR405 0x03a1b4b0
+#define MDIO_AN_VEND_PROV 0xc400
+#define MDIO_AN_VEND_PROV_1000BASET_FULL BIT(15)
+#define MDIO_AN_VEND_PROV_1000BASET_HALF BIT(14)
+
#define MDIO_AN_TX_VEND_STATUS1 0xc800
#define MDIO_AN_TX_VEND_STATUS1_10BASET (0x0 << 1)
#define MDIO_AN_TX_VEND_STATUS1_100BASETX (0x1 << 1)
@@ -64,10 +68,40 @@
static int aqr_config_aneg(struct phy_device *phydev)
{
- linkmode_copy(phydev->supported, phy_10gbit_features);
- linkmode_copy(phydev->advertising, phydev->supported);
+ bool changed = false;
+ u16 reg;
+ int ret;
- return 0;
+ if (phydev->autoneg == AUTONEG_DISABLE)
+ return genphy_c45_pma_setup_forced(phydev);
+
+ ret = genphy_c45_an_config_aneg(phydev);
+ if (ret < 0)
+ return ret;
+ if (ret > 0)
+ changed = true;
+
+ /* Clause 45 has no standardized support for 1000BaseT, therefore
+ * use vendor registers for this mode.
+ */
+ reg = 0;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ phydev->advertising))
+ reg |= MDIO_AN_VEND_PROV_1000BASET_FULL;
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+ phydev->advertising))
+ reg |= MDIO_AN_VEND_PROV_1000BASET_HALF;
+
+ ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_VEND_PROV,
+ MDIO_AN_VEND_PROV_1000BASET_HALF |
+ MDIO_AN_VEND_PROV_1000BASET_FULL, reg);
+ if (ret < 0)
+ return ret;
+ if (ret > 0)
+ changed = true;
+
+ return genphy_c45_check_and_restart_aneg(phydev, changed);
}
static int aqr_config_intr(struct phy_device *phydev)
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 3/5] net: phy: don't change modes we don't care about in genphy_c45_read_lpa
From: Heiner Kallweit @ 2019-02-22 22:50 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <4ae20155-367d-4b11-63f9-28580b7cf1cb@gmail.com>
Because 1000BaseT isn't covered by Clause 45, the 1000BaseT flags in
phydev->lp_advertising may have been set based on vendor registers
already. genphy_c45_read_lpa() would clear these flags as of today.
Therefore switch to mii_lpa_mod_linkmode_lpa_t.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy-c45.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index ce5fa5346..65ee33e34 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -268,7 +268,7 @@ int genphy_c45_read_lpa(struct phy_device *phydev)
if (val < 0)
return val;
- mii_lpa_to_linkmode_lpa_t(phydev->lp_advertising, val);
+ mii_lpa_mod_linkmode_lpa_t(phydev->lp_advertising, val);
phydev->pause = val & LPA_PAUSE_CAP ? 1 : 0;
phydev->asym_pause = val & LPA_PAUSE_ASYM ? 1 : 0;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 5/5] net: phy: aquantia: use genphy_c45_read_status
From: Heiner Kallweit @ 2019-02-22 22:52 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <4ae20155-367d-4b11-63f9-28580b7cf1cb@gmail.com>
Use new function genphy_c45_read_status(). 1000BaseT link partner
advertisement needs to be read from vendor registers.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/aquantia.c | 46 +++++++++++++++-----------------------
1 file changed, 18 insertions(+), 28 deletions(-)
diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index a1846daa3..0f0eb5682 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -39,6 +39,10 @@
#define MDIO_AN_TX_VEND_INT_MASK2 0xd401
#define MDIO_AN_TX_VEND_INT_MASK2_LINK BIT(0)
+#define MDIO_AN_RX_LP_STAT1 0xe820
+#define MDIO_AN_RX_LP_STAT1_1000BASET_FULL BIT(15)
+#define MDIO_AN_RX_LP_STAT1_1000BASET_HALF BIT(14)
+
/* Vendor specific 1, MDIO_MMD_VEND1 */
#define VEND1_GLOBAL_INT_STD_STATUS 0xfc00
#define VEND1_GLOBAL_INT_VEND_STATUS 0xfc01
@@ -154,36 +158,22 @@ static int aqr_ack_interrupt(struct phy_device *phydev)
static int aqr_read_status(struct phy_device *phydev)
{
- int reg;
-
- reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
- reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
- if (reg & MDIO_STAT1_LSTATUS)
- phydev->link = 1;
- else
- phydev->link = 0;
-
- reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_TX_VEND_STATUS1);
- mdelay(10);
- reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_TX_VEND_STATUS1);
-
- switch (reg & MDIO_AN_TX_VEND_STATUS1_RATE_MASK) {
- case MDIO_AN_TX_VEND_STATUS1_2500BASET:
- phydev->speed = SPEED_2500;
- break;
- case MDIO_AN_TX_VEND_STATUS1_1000BASET:
- phydev->speed = SPEED_1000;
- break;
- case MDIO_AN_TX_VEND_STATUS1_100BASETX:
- phydev->speed = SPEED_100;
- break;
- default:
- phydev->speed = SPEED_10000;
- break;
+ int val;
+
+ if (phydev->autoneg == AUTONEG_ENABLE) {
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_RX_LP_STAT1);
+ if (val < 0)
+ return val;
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ phydev->lp_advertising,
+ val & MDIO_AN_RX_LP_STAT1_1000BASET_FULL);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+ phydev->lp_advertising,
+ val & MDIO_AN_RX_LP_STAT1_1000BASET_HALF);
}
- phydev->duplex = DUPLEX_FULL;
- return 0;
+ return genphy_c45_read_status(phydev);
}
static int aqcs109_config_init(struct phy_device *phydev)
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 4/5] net: phy: add genphy_c45_read_status
From: Heiner Kallweit @ 2019-02-22 22:51 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <4ae20155-367d-4b11-63f9-28580b7cf1cb@gmail.com>
Similar to genphy_read_status() for Clause 22 add a generic read_status
function for Clause 45.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy-c45.c | 33 +++++++++++++++++++++++++++++++++
include/linux/phy.h | 1 +
2 files changed, 34 insertions(+)
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 65ee33e34..49e7cd08b 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -458,6 +458,39 @@ int genphy_c45_pma_read_abilities(struct phy_device *phydev)
}
EXPORT_SYMBOL_GPL(genphy_c45_pma_read_abilities);
+/**
+ * genphy_c45_read_status - read PHY status
+ * @phydev: target phy_device struct
+ *
+ * Reads status from PHY and sets phy_device members accordingly.
+ */
+int genphy_c45_read_status(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = genphy_c45_read_link(phydev);
+ if (ret)
+ return ret;
+
+ phydev->speed = SPEED_UNKNOWN;
+ phydev->duplex = DUPLEX_UNKNOWN;
+ phydev->pause = 0;
+ phydev->asym_pause = 0;
+
+ if (phydev->autoneg == AUTONEG_ENABLE) {
+ ret = genphy_c45_read_lpa(phydev);
+ if (ret)
+ return ret;
+
+ phy_resolve_aneg_linkmode(phydev);
+ } else {
+ ret = genphy_c45_read_pma(phydev);
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_read_status);
+
/* The gen10g_* functions are the old Clause 45 stub */
int gen10g_config_aneg(struct phy_device *phydev)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 8e9fc5764..a05ba366d 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1107,6 +1107,7 @@ int genphy_c45_an_config_aneg(struct phy_device *phydev);
int genphy_c45_an_disable_aneg(struct phy_device *phydev);
int genphy_c45_read_mdix(struct phy_device *phydev);
int genphy_c45_pma_read_abilities(struct phy_device *phydev);
+int genphy_c45_read_status(struct phy_device *phydev);
/* The gen10g_* functions are the old Clause 45 stub */
int gen10g_config_aneg(struct phy_device *phydev);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next 2/5] net: phy: aquantia: add support for auto-negotiation configuration
From: Florian Fainelli @ 2019-02-22 23:00 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <5eab0739-e38c-cb33-9314-881b51edcbf9@gmail.com>
On 2/22/19 2:49 PM, Heiner Kallweit wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> Make use of the generic c45 code, plus code specific to the Aquantia
> phy for 1000BaseT negotiation.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> drivers/net/phy/aquantia.c | 40 +++++++++++++++++++++++++++++++++++---
> 1 file changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
> index 9661ef4b4..a1846daa3 100644
> --- a/drivers/net/phy/aquantia.c
> +++ b/drivers/net/phy/aquantia.c
> @@ -20,6 +20,10 @@
> #define PHY_ID_AQCS109 0x03a1b5c2
> #define PHY_ID_AQR405 0x03a1b4b0
>
> +#define MDIO_AN_VEND_PROV 0xc400
> +#define MDIO_AN_VEND_PROV_1000BASET_FULL BIT(15)
> +#define MDIO_AN_VEND_PROV_1000BASET_HALF BIT(14)
> +
> #define MDIO_AN_TX_VEND_STATUS1 0xc800
> #define MDIO_AN_TX_VEND_STATUS1_10BASET (0x0 << 1)
> #define MDIO_AN_TX_VEND_STATUS1_100BASETX (0x1 << 1)
> @@ -64,10 +68,40 @@
>
> static int aqr_config_aneg(struct phy_device *phydev)
> {
> - linkmode_copy(phydev->supported, phy_10gbit_features);
> - linkmode_copy(phydev->advertising, phydev->supported);
> + bool changed = false;
You could entirely eliminate changed since this is just a shorthand for
ret > 0 after your recent changes to return that information. Other than
that:
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Jann Horn @ 2019-02-22 23:02 UTC (permalink / raw)
To: Nadav Amit
Cc: Andy Lutomirski, Alexei Starovoitov, Steven Rostedt,
Linus Torvalds, Masami Hiramatsu, Linux List Kernel Mailing,
Ingo Molnar, Andrew Morton, Changbin Du, Kees Cook,
Andy Lutomirski, Daniel Borkmann, Network Development,
bpf@vger.kernel.org, Rick Edgecombe, Dave Hansen,
Peter Zijlstra (Intel), Igor Stoppa
In-Reply-To: <A7232EDD-C53C-4B7D-AD02-4A480F40538B@vmware.com>
On Fri, Feb 22, 2019 at 11:39 PM Nadav Amit <namit@vmware.com> wrote:
> > On Feb 22, 2019, at 2:21 PM, Nadav Amit <namit@vmware.com> wrote:
> >
> >> On Feb 22, 2019, at 2:17 PM, Jann Horn <jannh@google.com> wrote:
> >>
> >> On Fri, Feb 22, 2019 at 11:08 PM Nadav Amit <namit@vmware.com> wrote:
> >>>> On Feb 22, 2019, at 1:43 PM, Jann Horn <jannh@google.com> wrote:
> >>>>
> >>>> (adding some people from the text_poke series to the thread, removing stable@)
> >>>>
> >>>> On Fri, Feb 22, 2019 at 8:55 PM Andy Lutomirski <luto@amacapital.net> wrote:
> >>>>>> On Feb 22, 2019, at 11:34 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >>>>>>> On Fri, Feb 22, 2019 at 02:30:26PM -0500, Steven Rostedt wrote:
> >>>>>>> On Fri, 22 Feb 2019 11:27:05 -0800
> >>>>>>> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >>>>>>>
> >>>>>>>>> On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
> >>>>>>>>>
> >>>>>>>>> Then we should still probably fix up "__probe_kernel_read()" to not
> >>>>>>>>> allow user accesses. The easiest way to do that is actually likely to
> >>>>>>>>> use the "unsafe_get_user()" functions *without* doing a
> >>>>>>>>> uaccess_begin(), which will mean that modern CPU's will simply fault
> >>>>>>>>> on a kernel access to user space.
> >>>>>>>>
> >>>>>>>> On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
> >>>>>>>> and users pass both user and kernel addresses into it and expect
> >>>>>>>> that the helper will actually try to read from that address.
> >>>>>>>>
> >>>>>>>> If __probe_kernel_read will suddenly start failing on all user addresses
> >>>>>>>> it will break the expectations.
> >>>>>>>> How do we solve it in bpf_probe_read?
> >>>>>>>> Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
> >>>>>>>> in the loop?
> >>>>>>>> That's doable, but people already complain that bpf_probe_read() is slow
> >>>>>>>> and shows up in their perf report.
> >>>>>>>
> >>>>>>> We're changing kprobes to add a specific flag to say that we want to
> >>>>>>> differentiate between kernel or user reads. Can this be done with
> >>>>>>> bpf_probe_read()? If it's showing up in perf report, I doubt a single
> >>>>>>
> >>>>>> so you're saying you will break existing kprobe scripts?
> >>>>>> I don't think it's a good idea.
> >>>>>> It's not acceptable to break bpf_probe_read uapi.
> >>>>>
> >>>>> If so, the uapi is wrong: a long-sized number does not reliably identify an address if you don’t separately know whether it’s a user or kernel address. s390x and 4G:4G x86_32 are the notable exceptions. I have lobbied for RISC-V and future x86_64 to join the crowd. I don’t know whether I’ll win this fight, but the uapi will probably have to change for at least s390x.
> >>>>>
> >>>>> What to do about existing scripts is a different question.
> >>>>
> >>>> This lack of logical separation between user and kernel addresses
> >>>> might interact interestingly with the text_poke series, specifically
> >>>> "[PATCH v3 05/20] x86/alternative: Initialize temporary mm for
> >>>> patching" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-6-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cf2513009ef734ecd6b0d08d69913a5ae%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864707020821793&sdata=HAbnDcrBne64JyPuVUMKmM7nQk67F%2BFvjuXEn8TmHeo%3D&reserved=0)
> >>>> and "[PATCH v3 06/20] x86/alternative: Use temporary mm for text
> >>>> poking" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-7-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cf2513009ef734ecd6b0d08d69913a5ae%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864707020821793&sdata=vNRIMKtFDy%2F3z5FlTwDiJY6VGEV%2FMHgQPTdFSFtCo4s%3D&reserved=0),
> >>>> right? If someone manages to get a tracing BPF program to trigger in a
> >>>> task that has switched to the patching mm, could they use
> >>>> bpf_probe_write_user() - which uses probe_kernel_write() after
> >>>> checking that KERNEL_DS isn't active and that access_ok() passes - to
> >>>> overwrite kernel text that is mapped writable in the patching mm?
> >>>
> >>> Yes, this is a good point. I guess text_poke() should be defined with
> >>> “__kprobes” and open-code memcpy.
> >>>
> >>> Does it sound reasonable?
> >>
> >> Doesn't __text_poke() as implemented in the proposed patch use a
> >> couple other kernel functions, too? Like switch_mm_irqs_off() and
> >> pte_clear() (which can be a call into a separate function on paravirt
> >> kernels)?
> >
> > I will move the pte_clear() to be done after the poking mm was unloaded.
> > Give me a few minutes to send a sketch of what I think should be done.
>
> Err.. You are right, I don’t see an easy way of preventing a kprobe from
> being set on switch_mm_irqs_off(), and open-coding this monster is too ugly.
>
> The reasonable solution seems to me as taking all the relevant pieces of
> code (and data) that might be used during text-poking and encapsulating them, so they
> will be set in a memory area which cannot be kprobe'd. This can also be
> useful to write-protect data structures of code that calls text_poke(),
> e.g., static-keys. It can also protect data on that stack that is used
> during text_poke() from being overwritten from another core.
>
> This solution is somewhat similar to Igor Stoppa’s idea of using “enclaves”
> when doing write-rarely operations.
>
> Right now, I think that text_poke() will keep being susceptible to such
> an attack, unless you have a better suggestion.
A relatively simple approach might be to teach BPF not to run kprobe
programs and such in contexts where current->mm isn't the active mm?
Maybe using nmi_uaccess_okay(), or something like that? It looks like
perf_callchain_user() also already uses that. Except that a lot of
this code is x86-specific...
^ permalink raw reply
* [PATCH bpf] bpf, doc: add bpf list as secondary entry to maintainers file
From: Daniel Borkmann @ 2019-02-22 23:03 UTC (permalink / raw)
To: alexei.starovoitov; +Cc: netdev, Daniel Borkmann
We recently created a bpf@vger.kernel.org list (https://lore.kernel.org/bpf/)
for BPF related discussions, originally in context of BPF track at LSF/MM
for topic discussions. It's *optional* but *desirable* to keep it in Cc for
BPF related kernel/loader/llvm/tooling threads, meaning also infrastructure
like llvm that sits on top of kernel but is crucial to BPF. In any case,
netdev with it's bpf delegate is *as-is* today primary list for patches, so
nothing changes in the workflow. Main purpose is to have some more awareness
for the bpf@vger.kernel.org list that folks can Cc for BPF specific topics.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
MAINTAINERS | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 41ce5f4..d78f371 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2852,7 +2852,7 @@ R: Martin KaFai Lau <kafai@fb.com>
R: Song Liu <songliubraving@fb.com>
R: Yonghong Song <yhs@fb.com>
L: netdev@vger.kernel.org
-L: linux-kernel@vger.kernel.org
+L: bpf@vger.kernel.org
T: git git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
T: git git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
Q: https://patchwork.ozlabs.org/project/netdev/list/?delegate=77147
@@ -2882,6 +2882,7 @@ N: bpf
BPF JIT for ARM
M: Shubham Bansal <illusionist.neo@gmail.com>
L: netdev@vger.kernel.org
+L: bpf@vger.kernel.org
S: Maintained
F: arch/arm/net/
@@ -2890,18 +2891,21 @@ M: Daniel Borkmann <daniel@iogearbox.net>
M: Alexei Starovoitov <ast@kernel.org>
M: Zi Shen Lim <zlim.lnx@gmail.com>
L: netdev@vger.kernel.org
+L: bpf@vger.kernel.org
S: Supported
F: arch/arm64/net/
BPF JIT for MIPS (32-BIT AND 64-BIT)
M: Paul Burton <paul.burton@mips.com>
L: netdev@vger.kernel.org
+L: bpf@vger.kernel.org
S: Maintained
F: arch/mips/net/
BPF JIT for NFP NICs
M: Jakub Kicinski <jakub.kicinski@netronome.com>
L: netdev@vger.kernel.org
+L: bpf@vger.kernel.org
S: Supported
F: drivers/net/ethernet/netronome/nfp/bpf/
@@ -2909,6 +2913,7 @@ BPF JIT for POWERPC (32-BIT AND 64-BIT)
M: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
M: Sandipan Das <sandipan@linux.ibm.com>
L: netdev@vger.kernel.org
+L: bpf@vger.kernel.org
S: Maintained
F: arch/powerpc/net/
@@ -2916,6 +2921,7 @@ BPF JIT for S390
M: Martin Schwidefsky <schwidefsky@de.ibm.com>
M: Heiko Carstens <heiko.carstens@de.ibm.com>
L: netdev@vger.kernel.org
+L: bpf@vger.kernel.org
S: Maintained
F: arch/s390/net/
X: arch/s390/net/pnet.c
@@ -2923,12 +2929,14 @@ X: arch/s390/net/pnet.c
BPF JIT for SPARC (32-BIT AND 64-BIT)
M: David S. Miller <davem@davemloft.net>
L: netdev@vger.kernel.org
+L: bpf@vger.kernel.org
S: Maintained
F: arch/sparc/net/
BPF JIT for X86 32-BIT
M: Wang YanQing <udknight@gmail.com>
L: netdev@vger.kernel.org
+L: bpf@vger.kernel.org
S: Maintained
F: arch/x86/net/bpf_jit_comp32.c
@@ -2936,6 +2944,7 @@ BPF JIT for X86 64-BIT
M: Alexei Starovoitov <ast@kernel.org>
M: Daniel Borkmann <daniel@iogearbox.net>
L: netdev@vger.kernel.org
+L: bpf@vger.kernel.org
S: Supported
F: arch/x86/net/
X: arch/x86/net/bpf_jit_comp32.c
@@ -8487,6 +8496,7 @@ L7 BPF FRAMEWORK
M: John Fastabend <john.fastabend@gmail.com>
M: Daniel Borkmann <daniel@iogearbox.net>
L: netdev@vger.kernel.org
+L: bpf@vger.kernel.org
S: Maintained
F: include/linux/skmsg.h
F: net/core/skmsg.c
@@ -16714,6 +16724,7 @@ M: Jesper Dangaard Brouer <hawk@kernel.org>
M: John Fastabend <john.fastabend@gmail.com>
L: netdev@vger.kernel.org
L: xdp-newbies@vger.kernel.org
+L: bpf@vger.kernel.org
S: Supported
F: net/core/xdp.c
F: include/net/xdp.h
@@ -16727,6 +16738,7 @@ XDP SOCKETS (AF_XDP)
M: Björn Töpel <bjorn.topel@intel.com>
M: Magnus Karlsson <magnus.karlsson@intel.com>
L: netdev@vger.kernel.org
+L: bpf@vger.kernel.org
S: Maintained
F: kernel/bpf/xskmap.c
F: net/xdp/
--
2.9.5
^ permalink raw reply related
* Re: [PATCH][net-next] ipv6: sanitize RCU usage on fib6_next
From: Eric Dumazet @ 2019-02-22 23:08 UTC (permalink / raw)
To: Li RongQing, netdev
In-Reply-To: <1550828682-10608-1-git-send-email-lirongqing@baidu.com>
On 02/22/2019 01:44 AM, Li RongQing wrote:
> using rcu_assign_pointer when setting, which has a memory
> barrier to ensure the initialization is seen first.
>
> using rcu_dereference when dereference this pointer
>
> Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
> net/ipv6/ip6_fib.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index 6613d8dbb0e5..b73d40d68178 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -1143,7 +1143,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
>
> atomic_inc(&rt->fib6_ref);
> rcu_assign_pointer(rt->fib6_node, fn);
> - rt->fib6_next = iter->fib6_next;
> + rcu_assign_pointer(rt->fib6_next, iter->fib6_next);
We do not need a barrier here, the object is still private.
> rcu_assign_pointer(*ins, rt);
> if (!info->skip_notify)
> inet6_rt_notify(RTM_NEWROUTE, rt, info, NLM_F_REPLACE);
> @@ -1761,7 +1761,9 @@ static void fib6_del_route(struct fib6_table *table, struct fib6_node *fn,
> RT6_TRACE("fib6_del_route\n");
>
> /* Unlink it */
> - *rtp = rt->fib6_next;
> + *rtp = rcu_dereference_protected(rt->fib6_next,
> + lockdep_is_held(&rt->fib6_table->tb6_lock));
This change will likely add a sparse bug.
> +
> rt->fib6_node = NULL;
> net->ipv6.rt6_stats->fib_rt_entries--;
> net->ipv6.rt6_stats->fib_discarded_routes++;
>
Really I do not believe these changes are needed.
barriers should be added blindly.
^ permalink raw reply
* Re: [PATCH bpf 0/2] nfp: bpf: fix two ALU32 code-gen bugs
From: Daniel Borkmann @ 2019-02-22 23:09 UTC (permalink / raw)
To: Jiong Wang, alexei.starovoitov; +Cc: netdev, oss-drivers
In-Reply-To: <1550874964-16769-1-git-send-email-jiong.wang@netronome.com>
On 02/22/2019 11:36 PM, Jiong Wang wrote:
> code-gen for BPF_ALU | BPF_XOR | BPF_K is wrong when imm is -1, also high
> 32-bit of 64-bit register should always be cleared.
>
> This set fixed both bugs.
>
> Jiong Wang (2):
> nfp: bpf: fix code-gen bug on BPF_ALU | BPF_XOR | BPF_K
> nfp: bpf: fix ALU32 high bits clearance bug
>
> drivers/net/ethernet/netronome/nfp/bpf/jit.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
Applied, thanks!
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox