* Re: [PATCH] staging: fsl-dpaa2/ethsw: Fix TCI values overwrite
From: Andrew Lunn @ 2018-03-27 13:37 UTC (permalink / raw)
To: Razvan Stefanescu
Cc: gregkh, devel, linux-kernel, netdev, alexandru.marginean,
ruxandra.radulescu, ioana.ciornei, laurentiu.tudor, stuyoder
In-Reply-To: <20180327131050.30581-1-razvan.stefanescu@nxp.com>
On Tue, Mar 27, 2018 at 08:10:50AM -0500, Razvan Stefanescu wrote:
> Previous implementation overwrites PCP value, assuming the default value is
> 0, instead of 7.
>
> Avoid this by modifying helper function ethsw_port_set_tci() to
> ethsw_port_set_pvid() and make it update only the vlan_id of the tci_cfg
> struct.
Hi Razvan
It is a good idea to explain acronyms, especially for staging, since
there are patches for all sorts of devices, can you cannot expect
everybody to know network specific acronyms.
By PCP you mean Priority Code Point. TCI i have no idea about.
Looking at the code, i think you are changing the flow to become
read/modify/write, instead of just write, which is overwriting the
previously configured Priority Code Point?
Please try to add more details to your change logs, to help us
understand the change.
Andrew
^ permalink raw reply
* Re: [PATCH iproute2-next] tc: Fix compilation error with old iptables
From: David Ahern @ 2018-03-27 13:40 UTC (permalink / raw)
To: Roi Dayan; +Cc: netdev, Stephen Hemminger, Eyal Birger
In-Reply-To: <1522142448-27697-1-git-send-email-roid@mellanox.com>
On 3/27/18 3:20 AM, Roi Dayan wrote:
> The compat_rev field does not exists in old versions of iptables.
> e.g. iptables 1.4.
>
> Fixes: dd29621578d2 ("tc: add em_ipt ematch for calling xtables matches from tc matching context")
> Signed-off-by: Roi Dayan <roid@mellanox.com>
> ---
> tc/em_ipt.c | 2 ++
> 1 file changed, 2 insertions(+)
>
Applied to iproute2-next. Thanks,
^ permalink raw reply
* Re: [net-next PATCH 3/5] net: netcp: ethss enhancements to support 2u cpsw h/w on K2G SoC
From: Andrew Lunn @ 2018-03-27 13:47 UTC (permalink / raw)
To: Murali Karicheri
Cc: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
linux-kernel, linux-arm-kernel, davem, netdev
In-Reply-To: <be58cf50-2148-cbfa-c7a2-8f4fbe6930e5@ti.com>
> Hello Andrew,
>
> Thanks for the comment. But I am not sure how to break this up as this is
> an enhancement to the driver to support a newer version of the cspw
> hardware. Without all these pieces together, the driver can't
> function.
Hi Murali
A few things to consider.
1) You can introduce new features needed by the new hardware one piece
at a time. The new hardware will then work when all the pieces have
been added. This is fine, the new hardware never worked before, so you
are not breaking anything.
2) Your changes could break the old hardware. By having lots of small
changes, you can do a git bissect and find which of the small changes
broke it.
3) It is much easier to review 10 small obviously correct patches than
one huge complex patch, which is not obvious.
> Probably I can make the below break up based on different functions
> added. Let me know if this looks good to you. Beware that patch #2 and
> #3 are small patches and majority of code change will be in patch #1
> which has to go together.
>
> Patch #1. Add support new link interface, RGMII_LINK_MAC_PHY, for K2G
> - Most of the code is for this
> Patch #2. Add support for configuring phy_mode
> - This just add phy_mode handling code
> Patch #3. map all vlan priorities to flow id 0
That sounds better.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH net] vhost: correctly remove wait queue during poll failure
From: Michael S. Tsirkin @ 2018-03-27 13:58 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <b1576bcf-652a-7c1a-9d77-db54931ab979@redhat.com>
On Tue, Mar 27, 2018 at 05:43:14PM +0800, Jason Wang wrote:
>
>
> On 2018年03月27日 17:28, Darren Kenny wrote:
> > Hi Jason,
> >
> > On Tue, Mar 27, 2018 at 11:47:22AM +0800, Jason Wang wrote:
> > > We tried to remove vq poll from wait queue, but do not check whether
> > > or not it was in a list before. This will lead double free. Fixing
> > > this by checking poll->wqh to make sure it was in a list.
> >
> > This text seems at odds with the code below, instead of checking
> > poll-whq, you are removing that check...
> >
> > Maybe the text needs rewording?
>
> Yes, I admit it's bad, thanks for pointing out.
>
> How about:
>
> "Fixing this by switching to use vhost_poll_stop() which zeros poll->wqh
> after removing poll from waitqueue to make sure it won't be freed twice."
>
> Thanks
Let's be a bit more specific about the problem maybe?
when vhost's attempt to start polling a descriptor fails, we remove the
poll->wqh entry from wait queue but do not clear it, so the following
cleanup (e.g. on release) will attempt to remove it again.
To fix, switch to vhost_poll_stop() which zeros poll->wqh
after removing poll from waitqueue to make sure it won't be freed twice."
the patch itself is fine though:
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > Thanks,
> >
> > Darren.
> >
> > >
> > > Reported-by: syzbot+c0272972b01b872e604a@syzkaller.appspotmail.com
> > > Fixes: 2b8b328b61c79 ("vhost_net: handle polling errors when setting
> > > backend")
> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > ---
> > > drivers/vhost/vhost.c | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > index 1b3e8d2d..5d5a9d9 100644
> > > --- a/drivers/vhost/vhost.c
> > > +++ b/drivers/vhost/vhost.c
> > > @@ -212,8 +212,7 @@ int vhost_poll_start(struct vhost_poll *poll,
> > > struct file *file)
> > > if (mask)
> > > vhost_poll_wakeup(&poll->wait, 0, 0, poll_to_key(mask));
> > > if (mask & EPOLLERR) {
> > > - if (poll->wqh)
> > > - remove_wait_queue(poll->wqh, &poll->wait);
> > > + vhost_poll_stop(poll);
> > > ret = -EINVAL;
> > > }
> > >
> > > --
> > > 2.7.4
> > >
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net V2] vhost: correctly remove wait queue during poll failure
From: Michael S. Tsirkin @ 2018-03-27 13:59 UTC (permalink / raw)
To: Jason Wang; +Cc: kvm, virtualization, netdev, linux-kernel, Darren Kenny
In-Reply-To: <1522155052-13347-1-git-send-email-jasowang@redhat.com>
On Tue, Mar 27, 2018 at 08:50:52PM +0800, Jason Wang wrote:
> We tried to remove vq poll from wait queue, but do not check whether
> or not it was in a list before. This will lead double free. Fixing
> this by switching to use vhost_poll_stop() which zeros poll->wqh after
> removing poll from waitqueue to make sure it won't be freed twice.
>
> Cc: Darren Kenny <darren.kenny@oracle.com>
> Reported-by: syzbot+c0272972b01b872e604a@syzkaller.appspotmail.com
> Fixes: 2b8b328b61c79 ("vhost_net: handle polling errors when setting backend")
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> Changes from V1:
> - tweak the commit log for to match the code
> ---
> drivers/vhost/vhost.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 1b3e8d2d..5d5a9d9 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -212,8 +212,7 @@ int vhost_poll_start(struct vhost_poll *poll, struct file *file)
> if (mask)
> vhost_poll_wakeup(&poll->wait, 0, 0, poll_to_key(mask));
> if (mask & EPOLLERR) {
> - if (poll->wqh)
> - remove_wait_queue(poll->wqh, &poll->wait);
> + vhost_poll_stop(poll);
> ret = -EINVAL;
> }
>
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH v7 0/7] netdev: Eliminate duplicate barriers on weakly-ordered archs
From: David Miller @ 2018-03-27 14:00 UTC (permalink / raw)
To: okaya; +Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel
In-Reply-To: <044d58fd-b6ea-6341-0cdf-d99c47fc241e@codeaurora.org>
From: Sinan Kaya <okaya@codeaurora.org>
Date: Tue, 27 Mar 2018 08:40:41 -0400
> Are you able to drop the applied ones so that I can post V7 or is it
> too late?
I cannot "drop" changes from my tree since my tree is used by thousands
of people and therefore immutable.
You must therefore send me relative fixes or reverts.
^ permalink raw reply
* Re: [PATCH v7 0/7] netdev: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-27 14:02 UTC (permalink / raw)
To: David Miller; +Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel
In-Reply-To: <20180327.100051.1676812408119987944.davem@davemloft.net>
On 3/27/2018 10:00 AM, David Miller wrote:
> From: Sinan Kaya <okaya@codeaurora.org>
> Date: Tue, 27 Mar 2018 08:40:41 -0400
>
>> Are you able to drop the applied ones so that I can post V7 or is it
>> too late?
>
> I cannot "drop" changes from my tree since my tree is used by thousands
> of people and therefore immutable.
>
> You must therefore send me relative fixes or reverts.
>
Thanks, I'll send fixes. Just wanted to see whether it got merged or if it was
sitting on a branch.
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [net-next PATCH 2/5] soc: ti: K2G: provide APIs to support driver probe deferral
From: Andrew Lunn @ 2018-03-27 14:03 UTC (permalink / raw)
To: Murali Karicheri
Cc: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
linux-kernel, linux-arm-kernel, davem, netdev
In-Reply-To: <4aeeda56-c68b-cb64-1314-a669f721ff20@ti.com>
> Could you please elaborate? These knav dma and qmss drivers are
> introduced to support packet DMA hardware available in Keystone
> NetCP which couldn't be implemented using the DMA APIs available
> at the time this driver was introduced. Another reason was that
> the performance was really bad. We had an internal implementation
> based on DMA API before which couldn't be upstreamed at that time
> due to the reason that we were mis-using the API for this driver.
> So we introduced these knav_dma driver to support NetCP. We don't
> have any plan to re-write the driver at this time.
>
> If your question is about EPROBE_DEFER being returned from an
> existing knav_dma API and using the return code to achieve probe
> defer instead of introducing these APIs, I can take a look into
> that and respond. So please clarify.
Hi Murali
So if i understood you right, at the time these drivers were written,
the linux DMA API did not do what you wanted. You could hack something
together by using the API wrongly, but that could not be mainlined. So
rather than fixing the DMA API to make it work for this hardware, you
ignored it, and made up your own API? This API now has its own
problems, it does not correctly handle ordering? So you are hacking
your own API further.
Does the Linux DMA API correctly handle probing order issues? Has the
Linux DMA API evolved so that it now does do what is needed by your
hardware?
If this was an old hardware which is slowly going away, it would not
be an issue. But it seems like there are new variants of the hardware
being released. So maybe you should go back and re-write the DMA
driver, rather than paper over the cracks?
Andrew
^ permalink raw reply
* Re: Re: [PATCH v7 0/7] netdev: intel: Eliminate duplicate barriers on weakly-ordered archs
From: Lino Sanfilippo @ 2018-03-27 14:04 UTC (permalink / raw)
To: Sinan Kaya
Cc: Jeff Kirsher, Alexander Duyck, intel-wired-lan, Netdev,
Timur Tabi, sulrich, linux-arm-msm, linux-arm-kernel
In-Reply-To: <e3ada376-52a5-573b-33f1-9aa84af75f0d@codeaurora.org>
Hi,
> Double sorry now.
>
> I don't know if you have been following "RFC on writel and writel_relaxed" thread
> or not but there are some new developments about wmb() requirement.
Just out of interest: Where can this thread be found?
>
> Basically, wmb() should never be used before writel() as writel() seem to
> provide coherency and observability guarantee.
>
AFAIU memory-barriers.txt writel() only guarantees correct order of accesses to
IO-memory not RAM vs. IO-memory (this may be the case for some architectures
where the writel() implementation contains a wmb() but not for all).
For the RAM vs. IO-memory case at least a a wmb()/rmb() has to be used.
Is this not correct?
Regards,
Lino
^ permalink raw reply
* Re: [net-next 00/10][pull request] 40GbE Intel Wired LAN Driver Updates 2018-03-26
From: David Miller @ 2018-03-27 14:05 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 26 Mar 2018 14:40:53 -0700
> This series contains updates to i40e only.
>
> Jake provides several patches which remove the need for cmpxchg64(),
> starting with moving I40E_FLAG_[UDP]_FILTER_SYNC from pf->flags to pf->state
> since they are modified during run time possibly when the RTNL lock is not
> held so they should be a state bits and not flags. Moved additional
> "flags" which should be state fields, into pf->state. Ensure we hold
> the RTNL lock for the entire sequence of preparing for reset and when
> resuming, which will protect the flags related to interrupt scheme under
> RTNL lock so that their modification is properly threaded. Finally,
> cleanup the use of cmpxchg64() since it is no longer needed. Cleaned up
> the holes in the feature flags created my moving some flags to the state
> field.
>
> Björn Töpel adds XDP_REDIRECT support as well as tweaking the page
> counting for XDP_REDIRECT so that it will function properly.
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [PATCH v6 bpf-next 07/11] tracepoint: introduce kernel_tracepoint_find_by_name
From: Steven Rostedt @ 2018-03-27 14:07 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, netdev, kernel-team, linux-api,
Mathieu Desnoyers
In-Reply-To: <20180327024706.2064725-8-ast@fb.com>
On Mon, 26 Mar 2018 19:47:02 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> introduce kernel_tracepoint_find_by_name() helper to let bpf core
> find tracepoint by name and later attach bpf probe to a tracepoint
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Thanks for doing this Alexei!
One nit below.
> ---
> include/linux/tracepoint.h | 6 ++++++
> kernel/tracepoint.c | 9 +++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index c92f4adbc0d7..a00b84473211 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -43,6 +43,12 @@ tracepoint_probe_unregister(struct tracepoint *tp, void *probe, void *data);
> extern void
> for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
> void *priv);
> +#ifdef CONFIG_TRACEPOINTS
> +struct tracepoint *kernel_tracepoint_find_by_name(const char *name);
> +#else
> +static inline struct tracepoint *
> +kernel_tracepoint_find_by_name(const char *name) { return NULL; }
> +#endif
>
> #ifdef CONFIG_MODULES
> struct tp_module {
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 671b13457387..e2a9a0391ae2 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -528,6 +528,15 @@ void for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
> }
> EXPORT_SYMBOL_GPL(for_each_kernel_tracepoint);
>
> +struct tracepoint *kernel_tracepoint_find_by_name(const char *name)
> +{
> + struct tracepoint * const *tp = __start___tracepoints_ptrs;
> +
> + for (; tp < __stop___tracepoints_ptrs; tp++)
> + if (!strcmp((*tp)->name, name))
> + return *tp;
Usually for cases like this, we prefer to add brackets for the for
block, as it's not a single line below it.
for (; tp < __stop__tracepoints_ptrs; tp++) {
if (!strcmp((*tp)->name, name))
return *tp;
}
-- Steve
> + return NULL;
> +}
> #ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS
>
> /* NB: reg/unreg are called while guarded with the tracepoints_mutex */
^ permalink raw reply
* Re: [PATCH v6 bpf-next 07/11] tracepoint: introduce kernel_tracepoint_find_by_name
From: Mathieu Desnoyers @ 2018-03-27 14:18 UTC (permalink / raw)
To: rostedt
Cc: Alexei Starovoitov, David S. Miller, Daniel Borkmann,
Linus Torvalds, Peter Zijlstra, netdev, kernel-team, linux-api
In-Reply-To: <20180327100711.0ec7aafb@gandalf.local.home>
----- On Mar 27, 2018, at 10:07 AM, rostedt rostedt@goodmis.org wrote:
> On Mon, 26 Mar 2018 19:47:02 -0700
> Alexei Starovoitov <ast@fb.com> wrote:
>
>> From: Alexei Starovoitov <ast@kernel.org>
>>
>> introduce kernel_tracepoint_find_by_name() helper to let bpf core
>> find tracepoint by name and later attach bpf probe to a tracepoint
>>
>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>
> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Steven showed preference for tracepoint_kernel_find_by_name() at some
point (starting with a tracepoint_ prefix). I'm find with either of
the names.
Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Thanks,
Mathieu
>
> Thanks for doing this Alexei!
>
> One nit below.
>
>
>> ---
>> include/linux/tracepoint.h | 6 ++++++
>> kernel/tracepoint.c | 9 +++++++++
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
>> index c92f4adbc0d7..a00b84473211 100644
>> --- a/include/linux/tracepoint.h
>> +++ b/include/linux/tracepoint.h
>> @@ -43,6 +43,12 @@ tracepoint_probe_unregister(struct tracepoint *tp, void
>> *probe, void *data);
>> extern void
>> for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
>> void *priv);
>> +#ifdef CONFIG_TRACEPOINTS
>> +struct tracepoint *kernel_tracepoint_find_by_name(const char *name);
>> +#else
>> +static inline struct tracepoint *
>> +kernel_tracepoint_find_by_name(const char *name) { return NULL; }
>> +#endif
>>
>> #ifdef CONFIG_MODULES
>> struct tp_module {
>> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
>> index 671b13457387..e2a9a0391ae2 100644
>> --- a/kernel/tracepoint.c
>> +++ b/kernel/tracepoint.c
>> @@ -528,6 +528,15 @@ void for_each_kernel_tracepoint(void (*fct)(struct
>> tracepoint *tp, void *priv),
>> }
>> EXPORT_SYMBOL_GPL(for_each_kernel_tracepoint);
>>
>> +struct tracepoint *kernel_tracepoint_find_by_name(const char *name)
>> +{
>> + struct tracepoint * const *tp = __start___tracepoints_ptrs;
>> +
>> + for (; tp < __stop___tracepoints_ptrs; tp++)
>> + if (!strcmp((*tp)->name, name))
>> + return *tp;
>
>
> Usually for cases like this, we prefer to add brackets for the for
> block, as it's not a single line below it.
>
> for (; tp < __stop__tracepoints_ptrs; tp++) {
> if (!strcmp((*tp)->name, name))
> return *tp;
> }
>
> -- Steve
>
>
>
>
>> + return NULL;
>> +}
>> #ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS
>>
> > /* NB: reg/unreg are called while guarded with the tracepoints_mutex */
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* [PATCH][next] ixgbevf: ensure xdp_ring resources are free'd on error exit
From: Colin King @ 2018-03-27 14:21 UTC (permalink / raw)
To: Jeff Kirsher, intel-wired-lan, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The current error handling for failed resource setup for xdp_ring
data is a break out of the loop and returning 0 indicated everying
was OK, when in fact it is not. Fix this by exiting via the
error exit label err_setup_tx that will clean up the resources
correctly and return and error status.
Detected by CoverityScan, CID#1466879 ("Logically dead code")
Fixes: 21092e9ce8b1 ("ixgbevf: Add support for XDP_TX action")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 3d9033f26eff..e3d04f226d57 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -3420,7 +3420,7 @@ static int ixgbevf_setup_all_tx_resources(struct ixgbevf_adapter *adapter)
if (!err)
continue;
hw_dbg(&adapter->hw, "Allocation for XDP Queue %u failed\n", j);
- break;
+ goto err_setup_tx;
}
return 0;
--
2.15.1
^ permalink raw reply related
* Re: [PATCH net] sctp: remove unnecessary asoc in sctp_has_association
From: David Miller @ 2018-03-27 14:22 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman
In-Reply-To: <955d9f51fafeb670cb4468e6253f4e54cd3c90f0.1522054500.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Mon, 26 Mar 2018 16:55:00 +0800
> After Commit dae399d7fdee ("sctp: hold transport instead of assoc
> when lookup assoc in rx path"), it put transport instead of asoc
> in sctp_has_association. Variable 'asoc' is not used any more.
>
> So this patch is to remove it, while at it, it also changes the
> return type of sctp_has_association to bool, and does the same
> for it's caller sctp_endpoint_is_peeled_off.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Since this is just a cleanup and a simplification, and doesn't fix
any bugs, I'm applying this to net-next.
Thanks.
^ permalink raw reply
* Re: [PATCH v7 0/7] netdev: intel: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-27 14:23 UTC (permalink / raw)
To: Lino Sanfilippo
Cc: Jeff Kirsher, Alexander Duyck, intel-wired-lan, Netdev,
Timur Tabi, sulrich, linux-arm-msm, linux-arm-kernel
In-Reply-To: <trinity-6d247cd1-94e3-4a58-a762-77724e895644-1522159460624@3c-app-gmx-bs61>
On 3/27/2018 10:04 AM, Lino Sanfilippo wrote:
> Hi,
>
>> Double sorry now.
>>
>> I don't know if you have been following "RFC on writel and writel_relaxed" thread
>> or not but there are some new developments about wmb() requirement.
>
> Just out of interest: Where can this thread be found?
https://www.spinics.net/lists/linux-rdma/msg62570.html
https://patchwork.kernel.org/patch/10309913/
>
>>
>> Basically, wmb() should never be used before writel() as writel() seem to
>> provide coherency and observability guarantee.
>>
>
> AFAIU memory-barriers.txt writel() only guarantees correct order of accesses to
> IO-memory not RAM vs. IO-memory (this may be the case for some architectures
> where the writel() implementation contains a wmb() but not for all).
> For the RAM vs. IO-memory case at least a a wmb()/rmb() has to be used.
> Is this not correct?
We are being told that if you use writel(), then you don't need a wmb() on
all architectures.
Jason is seeking behavior clarification for write combined buffers.
>
> Regards,
> Lino
>
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH v3 net 1/5] tcp: feed correct number of pkts acked to cc modules also in recovery
From: Ilpo Järvinen @ 2018-03-27 14:23 UTC (permalink / raw)
To: Yuchung Cheng; +Cc: netdev, Neal Cardwell, Eric Dumazet, Sergei Shtylyov
In-Reply-To: <CAK6E8=ed3Lt4seZNv+veaVT-U893FVHqWGWhzwfEXyeb-yAkhA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 7955 bytes --]
On Mon, 26 Mar 2018, Yuchung Cheng wrote:
> On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen
> <ilpo.jarvinen@helsinki.fi> wrote:
> >
> > A miscalculation for the number of acknowledged packets occurs during
> > RTO recovery whenever SACK is not enabled and a cumulative ACK covers
> > any non-retransmitted skbs. The reason is that pkts_acked value
> > calculated in tcp_clean_rtx_queue is not correct for slow start after
> > RTO as it may include segments that were not lost and therefore did
> > not need retransmissions in the slow start following the RTO. Then
> > tcp_slow_start will add the excess into cwnd bloating it and
> > triggering a burst.
> >
> > Instead, we want to pass only the number of retransmitted segments
> > that were covered by the cumulative ACK (and potentially newly sent
> > data segments too if the cumulative ACK covers that far).
> >
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> > ---
> > net/ipv4/tcp_input.c | 16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index 9a1b3c1..4a26c09 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -3027,6 +3027,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> > long seq_rtt_us = -1L;
> > long ca_rtt_us = -1L;
> > u32 pkts_acked = 0;
> > + u32 rexmit_acked = 0;
> > + u32 newdata_acked = 0;
> > u32 last_in_flight = 0;
> > bool rtt_update;
> > int flag = 0;
> > @@ -3056,8 +3058,10 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> > }
> >
> > if (unlikely(sacked & TCPCB_RETRANS)) {
> > - if (sacked & TCPCB_SACKED_RETRANS)
> > + if (sacked & TCPCB_SACKED_RETRANS) {
> > tp->retrans_out -= acked_pcount;
> > + rexmit_acked += acked_pcount;
> > + }
> > flag |= FLAG_RETRANS_DATA_ACKED;
> > } else if (!(sacked & TCPCB_SACKED_ACKED)) {
> > last_ackt = skb->skb_mstamp;
> > @@ -3070,6 +3074,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> > reord = start_seq;
> > if (!after(scb->end_seq, tp->high_seq))
> > flag |= FLAG_ORIG_SACK_ACKED;
> > + else
> > + newdata_acked += acked_pcount;
> > }
> >
> > if (sacked & TCPCB_SACKED_ACKED) {
> > @@ -3151,6 +3157,14 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> > }
> >
> > if (tcp_is_reno(tp)) {
> > + /* Due to discontinuity on RTO in the artificial
> > + * sacked_out calculations, TCP must restrict
> > + * pkts_acked without SACK to rexmits and new data
> > + * segments
> > + */
> > + if (icsk->icsk_ca_state == TCP_CA_Loss)
> > + pkts_acked = rexmit_acked + newdata_acked;
> > +
> My understanding is there are two problems
>
> 1) your fix: the reordering logic in tcp-remove_reno_sacks requires
> precise cumulatively acked count, not newly acked count?
While I'm not entirely sure if you intented to say that my fix is broken
or not, I thought this very difference alot while making the fix and I
believe that this fix is needed because of the discontinuity at RTO
(sacked_out is cleared as we set L-bits + lost_out). This is an artifact
in the imitation of sacked_out for non-SACK but at RTO we can't keep that
in sync because we set L-bits (and have no S-bits to guide us). Thus, we
cannot anymore "use" those skbs with only L-bit for the reno_sacks logic.
In tcp_remove_reno_sacks acked - sacked_out is being used to calculate
tp->delivered, using plain cumulative acked causes congestion control
breakage later as call to tcp_cong_control will directly use the
difference in tp->delivered.
This boils down the exact definition of tp->delivered (the one given in
the header is not detailed enough). I guess you might have better idea
what it exactly is since one of you has added it? There are subtle things
in the defination that can make it entirely unsuitable for cc decisions.
Should those segments that we (possibly) already counted into
tp->delivered during (potentially preceeding) CA_Recovery be added to it
for _second time_ or not? This fix avoids such double counting (the
price is that we might underestimate). For SACK+ACK losses, the similar
question is: Should those segments that we missed counting into
tp->delivered during preceeding CA_Recovery (due to losing enough SACKs)
be added into tp->delivered now during RTO recovery or not (I'm not
proposing we fix this unless we want to fix the both issues at the same
time here as its impact with SACK is not that significant)? Is
tp->delivered supposed to under- or overestimate (in the cases we're not
sure what/when something happened)? ...If it's overestimating under any
circumstances (for the current ACK), we cannot base our cc decision on it.
tcp_check_reno_reordering might like to have the cumulatively acked count
but due to the forementioned discontinuity we anyway cannot accurately
provide in CA_Loss what tcp_limit_reno_sacked expects (and the
other CA states are unaffected by this fix). While we could call
tcp_check_reno_reordering directly from tcp_clean_rtx_queue, it wouldn't
remove the fundamental discontinuity problem that we have for what
tcp_limit_reno_sacked assumes about sacked_out. It might actually be so
that tcp_limit_reno_sacked is just never going to work after we zero
tp->sacked_out (this would actually be the problem #3) or at least ends
up underestimating the reordering degree by the amount we cleared from
sacked_out at RTO?
> 2) current code: pkts_acked can substantially over-estimate the newly
> delivered pkts in both SACK and non-SACK cases. For example, let's say
> 99/100 packets are already sacked, and the next ACK acks 100 pkts.
> pkts_acked == 100 but really only one packet is delivered. It's wrong
> to inform congestion control that 100 packets have just delivered.
> AFAICT, the CCs that have pkts_acked callbacks all treat pkts_acked as
> the newly delivered packets.
>
> A better fix for both SACK and non-SACK, seems to be moving
> ca_ops->pkts_acked into tcp_cong_control, where the "acked_sacked" is
> calibrated? this is what BBR is currently doing to avoid these pitfalls.
Unfortunately that would not fix the problematic calculation of
tp->delivered.
But you might be right that there's a problem #2 (it's hard to notice
for real though as most of the cc modules don't seem to use it for
anything). However, this is for pkts_acked so are you sure what the
cc modules exactly expect: the shrinkage in # of outstanding packets or
the number of newly delivered packets?
Looking (only) quickly into the modules (that use it other than in
CA_Open):
- Somewhat suspicious delayed ACK detection logic in cdg
- HTCP might want shrinkage in # of outstanding packets (but I'm not
entirely sure) for throughput measurement
- I guess TCP illinois is broken (assumes it's newly delivered packets)
but it would not need to use it at all as it just proxies the value in
a local variable into tcp_illinois_cong_avoid that has the correct acked
readily available.
There are separate cong_avoid and cong_control callbacks that are more
obviously oriented for doing cc where as I'd think pkts_acked callback
seems more oriented to for RTT-sample related operations. Therefore, I'm
not entirely sure I this is what is wanted for pkts_acked, especially
given the HTCP example.
--
i.
^ permalink raw reply
* Re: [RFC PATCH v2] net: phy: Added device tree binding for dev-addr and dev-addr code check-up
From: Andrew Lunn @ 2018-03-27 14:24 UTC (permalink / raw)
To: Vicenţiu Galanopulo
Cc: Rob Herring, netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
mark.rutland@arm.com, davem@davemloft.net, marcel@holtmann.org,
devicetree@vger.kernel.org, Madalin-cristian Bucur,
Alexandru Marginean
In-Reply-To: <HE1PR0402MB3578B3D33160EBE59C2FD7D7EEAC0@HE1PR0402MB3578.eurprd04.prod.outlook.com>
> > This is a 2nd MDIO address, right? Can't you just append this to reg property?
Hi Rob
It is a sub address.
There are two different MDIO addressing schemes. Clause 22 allowed for
32 different addresses on an MDIO bus. Clause 45 extended that. You
have the existing 32 addresses for a package. However, within a
package, you can have 32 devices.
You are supposed to be able to look are registers inside the package,
and it will tell you which devices in the packages are in use. You can
then look at those devices and figure out what they are using ID
registers.
However some vendors get this wrong, they don't fill in the devices in
package information. So the generic probe code never finds them. We
need to pass it a hint, go looking at this specific device in the
package.
You can mix Clause 22 and Clause 45 on the same bus. Does DT allow two
different reg formats to be used at once? Can we have some reg
properties with a single value, and some with two values? I thought
#address-cells = <1> means there should be a single address in reg.
Andrew
^ permalink raw reply
* Re: [net-next PATCH 2/5] soc: ti: K2G: provide APIs to support driver probe deferral
From: Murali Karicheri @ 2018-03-27 14:31 UTC (permalink / raw)
To: Andrew Lunn
Cc: mark.rutland, devicetree, malat, netdev, linux-kernel, w-kwok2,
robh+dt, ssantosh, davem, linux-arm-kernel
In-Reply-To: <20180327140310.GK5862@lunn.ch>
Hello Andrew,
On 03/27/2018 10:03 AM, Andrew Lunn wrote:
>> Could you please elaborate? These knav dma and qmss drivers are
>> introduced to support packet DMA hardware available in Keystone
>> NetCP which couldn't be implemented using the DMA APIs available
>> at the time this driver was introduced. Another reason was that
>> the performance was really bad. We had an internal implementation
>> based on DMA API before which couldn't be upstreamed at that time
>> due to the reason that we were mis-using the API for this driver.
>> So we introduced these knav_dma driver to support NetCP. We don't
>> have any plan to re-write the driver at this time.
>>
>> If your question is about EPROBE_DEFER being returned from an
>> existing knav_dma API and using the return code to achieve probe
>> defer instead of introducing these APIs, I can take a look into
>> that and respond. So please clarify.
>
> Hi Murali
>
> So if i understood you right, at the time these drivers were written,
> the linux DMA API did not do what you wanted. You could hack something
> together by using the API wrongly, but that could not be mainlined. So
> rather than fixing the DMA API to make it work for this hardware, you
> ignored it, and made up your own API? This API now has its own
> problems, it does not correctly handle ordering? So you are hacking
> your own API further.
>
> Does the Linux DMA API correctly handle probing order issues? Has the
> Linux DMA API evolved so that it now does do what is needed by your
> hardware?
>
Thanks once again for your review and response!
I don't think dma API was meant to support hardware like pkt dma and was the
reason quoted when this driver was introduced and the same is valid even
today. AFAIK, Without hacking the API, we will not be able to support our
driver even today. Besides Keystone itself is an old platform that is
matured and have been there for long. K2G SoC was also introduced almost
3 years ago and we were late to port it to upstream due to various reasons.
We now have the SoC and most of the drivers upstreamed and this is a
missing driver to support networking. Given that we don't have any other
new devices planned from this SoC familty in the future, and the platform
itself is old and matured, I don't think the extra effort needed to
explore DMA API usage and re-write the driver is justified.
knav dma API is a TI SoC specific driver API and IMO, it should be fine\
to extend it to support probe deferral. I have also looked at the driver
and I don't see any other way to handle this since the channels are
allocated and used in ndo_open() and EPROBE_DEFER is not useful here.
Btw, I will re-write patch 3/5 as you have suggested as there is scope
for adding one more patch besides what I have mentioned in my response.
I will be sending v2 of the series soon with your comment on 3/5 addressed.
Regards,
Murali
> If this was an old hardware which is slowly going away, it would not
> be an issue. But it seems like there are new variants of the hardware
> being released. So maybe you should go back and re-write the DMA
> driver, rather than paper over the cracks?
>
> Andrew
>
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply
* Aw: Re: [PATCH v7 0/7] netdev: intel: Eliminate duplicate barriers on weakly-ordered archs
From: Lino Sanfilippo @ 2018-03-27 14:33 UTC (permalink / raw)
To: Sinan Kaya
Cc: Jeff Kirsher, Alexander Duyck, intel-wired-lan, Netdev,
Timur Tabi, sulrich, linux-arm-msm, linux-arm-kernel
In-Reply-To: <dcbd4406-73f7-4afa-850d-5f5d9f97ab02@codeaurora.org>
>
> On 3/27/2018 10:04 AM, Lino Sanfilippo wrote:
> > Hi,
> >
> >> Double sorry now.
> >>
> >> I don't know if you have been following "RFC on writel and writel_relaxed" thread
> >> or not but there are some new developments about wmb() requirement.
> >
> > Just out of interest: Where can this thread be found?
>
> https://www.spinics.net/lists/linux-rdma/msg62570.html
>
> https://patchwork.kernel.org/patch/10309913/
>
>
> >
> >>
> >> Basically, wmb() should never be used before writel() as writel() seem to
> >> provide coherency and observability guarantee.
> >>
> >
> > AFAIU memory-barriers.txt writel() only guarantees correct order of accesses to
> > IO-memory not RAM vs. IO-memory (this may be the case for some architectures
> > where the writel() implementation contains a wmb() but not for all).
> > For the RAM vs. IO-memory case at least a a wmb()/rmb() has to be used.
> > Is this not correct?
>
> We are being told that if you use writel(), then you don't need a wmb() on
> all architectures.
>
> Jason is seeking behavior clarification for write combined buffers.
>
Interesting, thanks for the information!
Lino
^ permalink raw reply
* Re: linux-next: manual merge of the ipsec tree with the net tree
From: Petr Machata @ 2018-03-27 14:33 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Steffen Klassert, David Miller, Networking,
Linux-Next Mailing List, Linux Kernel Mailing List,
Stefano Brivio
In-Reply-To: <20180326091610.148fc62d@canb.auug.org.au>
Stephen Rothwell <sfr@canb.auug.org.au> writes:
> Hi Steffen,
>
> Today's linux-next merge of the ipsec tree got a conflict in:
>
> net/ipv4/ip_tunnel.c
>
> between commit:
>
> f6cc9c054e77 ("ip_tunnel: Emit events for post-register MTU changes")
>
> from the net tree and commit:
>
> 24fc79798b8d ("ip_tunnel: Clamp MTU to bounds on new link")
>
> from the ipsec tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
Hi, this conflict needs to be resolved differently. Below I'm adding a
patch on top of linux-next. Squash or apply as you see fit.
What's the right way to proceed from here? It looks to me like Stefano
or Steffen should take this into the ipsec tree (possibly just squash to
the clamping fix), as they'll hit this conflict the next time they
rebase on top of net...? Let me know what to do, please.
Thanks,
Petr
>From 2c2210e59cd94571ac67d491026d6ea0f4d69ae4 Mon Sep 17 00:00:00 2001
Message-Id: <2c2210e59cd94571ac67d491026d6ea0f4d69ae4.1522155963.git.petrm@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
Date: Tue, 27 Mar 2018 16:00:38 +0300
Subject: [PATCH linux-next] net: ip_tunnel: Call dev_set_mtu to set
clamped MTU
When adding a new IP tunnel link, MTU is clamped to sane bounds.
However, since the new MTU value is directly assigned to the netdevice,
instead of going through dev_set_mtu, the change is done without sending
the usual NETDEV_PRECHANGEMTU and NETDEV_CHANGEMTU events. Thus drivers
don't get to learn about the clamped MTU value.
Signed-off-by: Petr Machata <petrm@mellanox.com>
---
net/ipv4/ip_tunnel.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index a57cf0d..de6d944 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -1108,14 +1108,14 @@ int ip_tunnel_newlink(struct net_device *dev, struct nlattr *tb[],
if (tb[IFLA_MTU]) {
unsigned int max = 0xfff8 - dev->hard_header_len - nt->hlen;
- dev->mtu = clamp(dev->mtu, (unsigned int)ETH_MIN_MTU,
- (unsigned int)(max - sizeof(struct iphdr)));
- } else {
- err = dev_set_mtu(dev, mtu);
- if (err)
- goto err_dev_set_mtu;
+ mtu = clamp(dev->mtu, (unsigned int)ETH_MIN_MTU,
+ (unsigned int)(max - sizeof(struct iphdr)));
}
+ err = dev_set_mtu(dev, mtu);
+ if (err)
+ goto err_dev_set_mtu;
+
ip_tunnel_add(itn, nt);
return 0;
--
2.4.11
^ permalink raw reply related
* Re: [PATCH v7 0/7] netdev: intel: Eliminate duplicate barriers on weakly-ordered archs
From: Alexander Duyck @ 2018-03-27 14:38 UTC (permalink / raw)
To: Sinan Kaya
Cc: Lino Sanfilippo, Jeff Kirsher, intel-wired-lan, Netdev,
Timur Tabi, sulrich, linux-arm-msm, linux-arm-kernel
In-Reply-To: <dcbd4406-73f7-4afa-850d-5f5d9f97ab02@codeaurora.org>
On Tue, Mar 27, 2018 at 7:23 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
> On 3/27/2018 10:04 AM, Lino Sanfilippo wrote:
>> Hi,
>>
>>> Double sorry now.
>>>
>>> I don't know if you have been following "RFC on writel and writel_relaxed" thread
>>> or not but there are some new developments about wmb() requirement.
>>
>> Just out of interest: Where can this thread be found?
>
> https://www.spinics.net/lists/linux-rdma/msg62570.html
>
> https://patchwork.kernel.org/patch/10309913/
>
>
>>
>>>
>>> Basically, wmb() should never be used before writel() as writel() seem to
>>> provide coherency and observability guarantee.
>>>
>>
>> AFAIU memory-barriers.txt writel() only guarantees correct order of accesses to
>> IO-memory not RAM vs. IO-memory (this may be the case for some architectures
>> where the writel() implementation contains a wmb() but not for all).
>> For the RAM vs. IO-memory case at least a a wmb()/rmb() has to be used.
>> Is this not correct?
>
> We are being told that if you use writel(), then you don't need a wmb() on
> all architectures.
I'm not sure who told you that but that is incorrect, at least for
x86. If you attempt to use writel() without the wmb() we will have to
NAK the patches. We will accept the wmb() with writel_releaxed() since
that solves things for ARM.
> Jason is seeking behavior clarification for write combined buffers.
Don't bother. I can tell you right now that for x86 you have to have a
wmb() before the writel().
Based on the comment in
(https://www.spinics.net/lists/linux-rdma/msg62666.html):
Replacing wmb() + writel() with wmb() + writel_relaxed() will work on
PPC, it will just not give you a benefit today.
I say the patch set stays. This gives benefit on ARM, and has no
effect on x86 and PowerPC. If you want to look at trying to optimize
things further on PowerPC and such then go for it in terms of trying
to implement the writel_relaxed(). Otherwise I say we call the ARM
goodness a win and don't get ourselves too wrapped up in trying to fix
this for all architectures.
Thanks.
- Alex
^ permalink raw reply
* Re: [PATCH] net/ncsi: check for null return from call to nla_nest_start
From: David Miller @ 2018-03-27 14:39 UTC (permalink / raw)
To: colin.king; +Cc: dan.carpenter, sam, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20180326112712.31165-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Mon, 26 Mar 2018 12:27:12 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> The call to nla_nest_start calls nla_put which can lead to a NULL
> return so it's possible for attr to become NULL and we can potentially
> get a NULL pointer dereference on attr. Fix this by checking for
> a NULL return.
>
> Detected by CoverityScan, CID#1466125 ("Dereference null return")
>
> Fixes: 955dc68cb9b2 ("net/ncsi: Add generic netlink family")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied, but...
Colin, please start marking your Subject lines properly with "[PATCH
net-next]" for patches targetting net-next as opposed to net.
Thank you.
^ permalink raw reply
* Re: [PATCH 0/2] pull request for net: batman-adv 2018-03-26
From: David Miller @ 2018-03-27 14:40 UTC (permalink / raw)
To: sw; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <20180326124754.5897-1-sw@simonwunderlich.de>
From: Simon Wunderlich <sw@simonwunderlich.de>
Date: Mon, 26 Mar 2018 14:47:52 +0200
> here are two late bugfixes for batman-adv which we would like to have in net
> if still possible.
>
> Please pull or let me know of any problem!
Pulled, thank you Simon.
^ permalink raw reply
* Re: [PATCH v6 bpf-next 07/11] tracepoint: introduce kernel_tracepoint_find_by_name
From: Steven Rostedt @ 2018-03-27 14:42 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Alexei Starovoitov, David S. Miller, Daniel Borkmann,
Linus Torvalds, Peter Zijlstra, netdev, kernel-team, linux-api
In-Reply-To: <1566743692.1117.1522160304471.JavaMail.zimbra@efficios.com>
On Tue, 27 Mar 2018 10:18:24 -0400 (EDT)
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> ----- On Mar 27, 2018, at 10:07 AM, rostedt rostedt@goodmis.org wrote:
>
> > On Mon, 26 Mar 2018 19:47:02 -0700
> > Alexei Starovoitov <ast@fb.com> wrote:
> >
> >> From: Alexei Starovoitov <ast@kernel.org>
> >>
> >> introduce kernel_tracepoint_find_by_name() helper to let bpf core
> >> find tracepoint by name and later attach bpf probe to a tracepoint
> >>
> >> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> >
> > Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>
> Steven showed preference for tracepoint_kernel_find_by_name() at some
> point (starting with a tracepoint_ prefix). I'm find with either of
> the names.
Yeah, I do prefer tracepoint_kernel_find_by_name() to stay consistent
with the other tracepoint functions. But we have
"for_each_kernel_tracepoint()" and not "for_each_tracepoint_kernel()",
thus we need to pick being consistent with one or the other. One answer
is to use tracpoint_kernel_find_by_name() and rename the for_each to
for_each_tracpoint_kernel().
-- Steve
>
> Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>
> Thanks,
>
> Mathieu
>
> >
> > Thanks for doing this Alexei!
> >
> > One nit below.
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Jason Gunthorpe @ 2018-03-27 14:44 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Steve Wise, David Ahern, stephen, netdev, linux-rdma
In-Reply-To: <20180327032141.GS1877@mtr-leonro.local>
On Tue, Mar 27, 2018 at 06:21:41AM +0300, Leon Romanovsky wrote:
> On Mon, Mar 26, 2018 at 04:30:33PM -0600, Jason Gunthorpe wrote:
> > On Mon, Mar 26, 2018 at 04:34:44PM -0500, Steve Wise wrote:
> > >
> > > On 3/26/2018 4:15 PM, Jason Gunthorpe wrote:
> > > > On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise wrote:
> > > >>
> > > >> On 3/26/2018 9:17 AM, David Ahern wrote:
> > > >>> On 2/27/18 9:07 AM, Steve Wise wrote:
> > > >>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
> > > >>>> index 5809f70..e55205b 100644
> > > >>>> +++ b/rdma/rdma.h
> > > >>>> @@ -18,10 +18,12 @@
> > > >>>> #include <libmnl/libmnl.h>
> > > >>>> #include <rdma/rdma_netlink.h>
> > > >>>> #include <time.h>
> > > >>>> +#include <net/if_arp.h>
> > > >>>>
> > > >>>> #include "list.h"
> > > >>>> #include "utils.h"
> > > >>>> #include "json_writer.h"
> > > >>>> +#include <rdma/rdma_cma.h>
> > > >>>>
> > > >>> did you forget to add rdma_cma.h? I don't see that file in my repo.
> > > >> It is provided by the rdma-core package, upon which rdma tool now
> > > >> depends for the rdma_port_space enum.
> > > > It is a kernel bug that enum is not in an include/uapi/rdma header
> > > >
> > > > Fix it there and don't try to use rdma-core headers to get kernel ABI.
> > > >
> > > > Jason
> > >
> > > I wish you'd commented on this just a little sooner. I just resent v3
> > > of this series... with rdma_cma.h included. :)
> > >
> > > How about the restrack/nldev code just translates the port space from
> > > enum rdma_port_space to a new ABI enum, say nldev_rdma_port_space, that
> > > i add to rdma_netlink.h? I'd hate to open the can of worms of trying to
> > > split rdma_cma.h into uabi and no uabi headers. :(
> >
> > If port space is already part of the ABI there isn't much reason to
> > translate it.
> >
> > You just need to pick the right header to put it in, since it is a verbs
> > define it doesn't belong in the netlink header.
>
> I completely understand Steve's concerns.
>
> I tried to do such thing (expose kernel headers) in first incarnation of
> rdmatool with attempt to clean IB/core as well to ensure that we won't expose
> anything that is not implemented. It didn't go well.
rdma-core is now using the kernel uapi/ headers natively, seems to be
going OK. What problem did you face?
Jason
^ 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