* Re: [net-next PATCH v5 4/7] net: Record receive queue number for a connection
From: David Miller @ 2018-06-29 13:06 UTC (permalink / raw)
To: amritha.nambiar
Cc: netdev, alexander.h.duyck, willemdebruijn.kernel,
sridhar.samudrala, alexander.duyck, edumazet, hannes, tom, tom
In-Reply-To: <153013869437.4959.6403551024336066798.stgit@anamhost.jf.intel.com>
From: Amritha Nambiar <amritha.nambiar@intel.com>
Date: Wed, 27 Jun 2018 15:31:34 -0700
> @@ -1702,6 +1709,13 @@ static inline int sk_tx_queue_get(const struct sock *sk)
> return -1;
> }
>
> +static inline void sk_rx_queue_set(struct sock *sk, const struct sk_buff *skb)
> +{
> +#ifdef CONFIG_XPS
> + sk->sk_rx_queue_mapping = skb_get_rx_queue(skb);
> +#endif
> +}
Maybe add a >= NO_QUEUE_MAPPING check like you did for
skc_tx_queue_mapping.
^ permalink raw reply
* Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: Jiri Pirko @ 2018-06-29 13:05 UTC (permalink / raw)
To: David Ahern
Cc: Jamal Hadi Salim, Cong Wang, Linux Kernel Network Developers,
David Miller, Jakub Kicinski, Simon Horman, john.hurley, mlxsw,
sridhar.samudrala
In-Reply-To: <95856cb5-62bc-5b1d-6469-bb72c886891a@gmail.com>
Fri, Jun 29, 2018 at 02:54:36PM CEST, dsahern@gmail.com wrote:
>On 6/29/18 6:48 AM, Jiri Pirko wrote:
>> Fri, Jun 29, 2018 at 02:12:21PM CEST, jhs@mojatatu.com wrote:
>>> On 29/06/18 04:39 AM, Jiri Pirko wrote:
>>>> Fri, Jun 29, 2018 at 12:25:53AM CEST, xiyou.wangcong@gmail.com wrote:
>>>>> On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>>>>> Add a template of type flower allowing to insert rules matching on last
>>>>>> 2 bytes of destination mac address:
>>>>>> # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>>>>>>
>>>>>> The template is now showed in the list:
>>>>>> # tc chaintemplate show dev dummy0 ingress
>>>>>> chaintemplate flower chain 0
>>>>>> dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>>>> eth_type ipv4
>>>>>>
>>>>>> Add another template, this time for chain number 22:
>>>>>> # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
>>>>>> # tc chaintemplate show dev dummy0 ingress
>>>>>> chaintemplate flower chain 0
>>>>>> dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>>>> eth_type ipv4
>>>>>> chaintemplate flower chain 22
>>>>>> eth_type ipv4
>>>>>> dst_ip 0.0.0.0/16
>>>>>
>>>>> So, if I want to check the template of a chain, I have to use
>>>>> 'tc chaintemplate... chain X'.
>>>>>
>>>>> If I want to check the filters in a chain, I have to use
>>>>> 'tc filter show .... chain X'.
>>>>>
>>>>> If you introduce 'tc chain', it would just need one command:
>>>>> `tc chain show ... X` which could list its template first and
>>>>> followed by filters in this chain, something like:
>>>>>
>>>>> # tc chain show dev eth0 chain X
>>>>> template: # could be none
>>>>> ....
>>>>> filter1
>>>>> ...
>>>>> filter2
>>>>> ...
>>>>>
>>>>> Isn't it more elegant?
>>>>
>>>> Well, that is just another iproute2 command. It would use the same
>>>> kernel uapi. Filters+templates. Sure, why not. Can be easily introduced.
>>>> Let's do it in a follow-up iproute2 patch.
>>>>
>>>
>>> Half a dozen or 6 - take your pick, really.
>>> I would call the template an attribute as opposed to a stand alone
>>> object i.e A chain of filters may have a template. If you have to
>>> introduce a new object then Sridhar's suggested syntax seems appealing.
>>
>> I think what I have makes sense. Maps nicely to what it really is inside
>> kernel. What Cong proposes looks like nice extension of userspace app to
>> do more things in one go. Will address that in followup iproute2 patch.
>
>The resolution of the syntax affect the uapi changes proposed. You are
>wanting to add new RTM commands which suggests new objects. If a
>template is an attribute of an existing object then the netlink API
>should indicate that.
There is no existing "chain" object. So no, no uapi changes.
^ permalink raw reply
* Re: pull-request: mac80211 2018-06-29
From: David Miller @ 2018-06-29 13:09 UTC (permalink / raw)
To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <20180629074852.20096-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri, 29 Jun 2018 09:48:51 +0200
> Hi Dave,
>
> For the current release, I have a few fixes. No sense waiting
> for more, and most of these have been around for a while.
>
> Please pull and let me know if there's any problem.
...
> git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-davem-2018-06-29
Pulled, thanks a lot.
^ permalink raw reply
* Re: [PATCH 1/3] ath10k: sdio: use same endpoint id for all packets in a bundle
From: Niklas Cassel @ 2018-06-29 13:11 UTC (permalink / raw)
To: Kalle Valo
Cc: alagusankar, netdev, linux-wireless, linux-kernel, ath10k,
David S. Miller
In-Reply-To: <87vaa1erlj.fsf@codeaurora.org>
On Fri, Jun 29, 2018 at 02:53:44PM +0300, Kalle Valo wrote:
> Kalle Valo <kvalo@codeaurora.org> writes:
>
> > Niklas Cassel <niklas.cassel@linaro.org> writes:
> >
> >> All packets in a bundle should use the same endpoint id as the
> >> first lookahead.
> >>
> >> This matches how things are done is ath6kl, however,
> >> this patch can theoretically handle several bundles
> >> in ath10k_sdio_mbox_rx_process_packets().
> >>
> >> Without this patch we get lots of errors about invalid endpoint id:
> >>
> >> ath10k_sdio mmc2:0001:1: invalid endpoint in look-ahead: 224
> >> ath10k_sdio mmc2:0001:1: failed to get pending recv messages: -12
> >> ath10k_sdio mmc2:0001:1: failed to process pending SDIO interrupts: -12
> >>
> >> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
> >> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> >
> > You have Alagu's s-o-b first so that implies Alagu is the author. So
> > should I add From header for Alagu and add you (Niklas) as
> > Co-Developed-by? Or vice versa?
Hello Kalle,
It is not always obvious how the combination of git-author
and Co-Developed-by should be:
http://lkml.iu.edu/hypermail/linux/kernel/1801.2/00988.html
Alagu deserves most credit, since I have simply taken parts
of a very big patch and split it into smaller pieces.
I tried to do the right thing by having his Signed-off-by first.
Let's go with your suggestion and add a From: header with
Alagu's email, and a Co-Developed-by tag with my email.
(Note that both Signed-off-bys are still needed according to
submitting-patches.rst)
Do you want me to resend the patches with these two lines
added, or can you fix them up manually?
Regards,
Niklas
>
> Ah, the same issue is with all three patches. So whatever we decide,
> I'll do the same changes on all three.
>
> --
> Kalle Valo
^ permalink raw reply
* [PATCH bpf] bpf: hash_map: decrement counter on error
From: Mauricio Vasquez B @ 2018-06-29 12:48 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev
Decrement the number of elements in the map in case the allocation
of a new node fails.
Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>
---
kernel/bpf/hashtab.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 3ca2198..513d9df 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -747,13 +747,15 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
* old element will be freed immediately.
* Otherwise return an error
*/
- atomic_dec(&htab->count);
- return ERR_PTR(-E2BIG);
+ l_new = ERR_PTR(-E2BIG);
+ goto dec_count;
}
l_new = kmalloc_node(htab->elem_size, GFP_ATOMIC | __GFP_NOWARN,
htab->map.numa_node);
- if (!l_new)
- return ERR_PTR(-ENOMEM);
+ if (!l_new) {
+ l_new = ERR_PTR(-ENOMEM);
+ goto dec_count;
+ }
}
memcpy(l_new->key, key, key_size);
@@ -766,7 +768,8 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
GFP_ATOMIC | __GFP_NOWARN);
if (!pptr) {
kfree(l_new);
- return ERR_PTR(-ENOMEM);
+ l_new = ERR_PTR(-ENOMEM);
+ goto dec_count;
}
}
@@ -780,6 +783,9 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
l_new->hash = hash;
return l_new;
+dec_count:
+ atomic_dec(&htab->count);
+ return l_new;
}
static int check_flags(struct bpf_htab *htab, struct htab_elem *l_old,
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] tg3: Mark expected switch fall-throughs
From: David Miller @ 2018-06-29 13:18 UTC (permalink / raw)
To: gustavo; +Cc: siva.kallam, prashant, mchan, netdev, linux-kernel
In-Reply-To: <20180628014524.GA26061@embeddedor.com>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Wed, 27 Jun 2018 20:45:24 -0500
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH v4] net: ethernet: stmmac: dwmac-rk: Add GMAC support for px30
From: David Miller @ 2018-06-29 13:19 UTC (permalink / raw)
To: david.wu
Cc: heiko, robh+dt, mark.rutland, huangtao, netdev, linux-arm-kernel,
linux-rockchip, linux-kernel
In-Reply-To: <1530149601-23337-1-git-send-email-david.wu@rock-chips.com>
From: David Wu <david.wu@rock-chips.com>
Date: Thu, 28 Jun 2018 09:33:21 +0800
> Add constants and callback functions for the dwmac on px30 Soc.
> The base structure is the same, but registers and the bits in
> them are moved slightly, and add the clk_mac_speed for selecting
> mac speed.
>
> Signed-off-by: David Wu <david.wu@rock-chips.com>
Applied.
^ permalink raw reply
* Re: [PATCHv3 net-next] sctp: add support for SCTP_REUSE_PORT sockopt
From: David Miller @ 2018-06-29 13:21 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman
In-Reply-To: <710f2cfa644383f8455d9612a80347a28e00d113.1530171060.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Thu, 28 Jun 2018 15:31:00 +0800
> This feature is actually already supported by sk->sk_reuse which can be
> set by socket level opt SO_REUSEADDR. But it's not working exactly as
> RFC6458 demands in section 8.1.27, like:
>
> - This option only supports one-to-one style SCTP sockets
> - This socket option must not be used after calling bind()
> or sctp_bindx().
>
> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> work in linux.
>
> To separate it from the socket level version, this patch adds 'reuse' in
> sctp_sock and it works pretty much as sk->sk_reuse, but with some extra
> setup limitations that are needed when it is being enabled.
>
> "It should be noted that the behavior of the socket-level socket option
> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> leaves SO_REUSEADDR as is for the compatibility.
>
> Note that the name SCTP_REUSE_PORT is somewhat confusing, as its
> functionality is nearly identical to SO_REUSEADDR, but with some
> extra restrictions. Here it uses 'reuse' in sctp_sock instead of
> 'reuseport'. As for sk->sk_reuseport support for SCTP, it will be
> added in another patch.
>
> Thanks to Neil to make this clear.
>
> v1->v2:
> - add sctp_sk->reuse to separate it from the socket level version.
> v2->v3:
> - improve changelog according to Marcelo's suggestion.
>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied, thanks for working on getting the semantics right.
^ permalink raw reply
* Re: Link modes representation in phylib
From: Maxime Chevallier @ 2018-06-29 13:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: davem, Russell King - ARM Linux, Florian Fainelli, netdev,
Antoine Tenart, thomas.petazzoni@bootlin.com, Gregory CLEMENT,
Miquel Raynal
In-Reply-To: <20180619152155.GC26796@lunn.ch>
Hello Andrew,
On Tue, 19 Jun 2018 17:21:55 +0200
Andrew Lunn <andrew@lunn.ch> wrote:
>> What I propose is that we add 3 link_mode fields in phy_device, and keep
>> the legacy fields for now. It would be up to the driver to fill the new
>> "supported" field in config_init, kind of like what's done in the
>> marvell10g driver.
>
>Hi Maxime
>
>You can do this conversion in the core. If features == 0, and some
>bits are set in the features link_mode, do the conversion at probe
>time. The same can be done for lp_advertising, when the call into the
>drivers read_status() has completed.
Thanks for the suggestion. I see how this can be done with
phydrv->supported and phydev->lp_advertising, however I'm not sure how
we should deal with the fact that ethernet drivers directly access
fields such as "advertising" or "supported".
Should we update the new fields in phy_device when functions such as
"phy_start" or "phy_start_aneg" are called, just in case the ethernet
driver modified the phydev->supported / phydev->advertising fields
in the meantime ?
My concern is that phylink will rely on the new link_mode
representation to be up-to-date, and ethernet drivers that aren't
converted to phylink, but link to a new PHY will rely on the legacy
representation.
That might be just fine if we take good care making sure both the
legacy and the new representation are well sync'd, but since I don't
have a good big-picture view of all this, I prefer having your opinion
first :)
>> Would that be acceptable ?
>
>It sounds reasonable. Lets see what the code looks like.
Ok I'll be glad to send a series for that, I just want to make sure I
go in the right direction
Thanks,
Maxime
^ permalink raw reply
* Re: [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer
From: Christoph Hellwig @ 2018-06-29 13:28 UTC (permalink / raw)
To: Linus Torvalds
Cc: Al Viro, Christoph Hellwig, linux-fsdevel, Network Development,
LKP
In-Reply-To: <CA+55aFxfkD9ozw77j-bshReLJN4dN2GEdW+R-LkxztceCW6OTg@mail.gmail.com>
On Thu, Jun 28, 2018 at 02:11:17PM -0700, Linus Torvalds wrote:
> Christoph, do you have a test program for IOCB_CMD_POLL and what it's
> actually supposed to do?
https://pagure.io/libaio/c/9c6935e81854d1585bbfa48c35b185849d746864?branch=aio-poll
is the actual test in libaio. In addition to that the seastar library
actually has a real life user. But given that is c++ with all modern
bells and whistles you'll probably have an as hard time as me actually
understanding that.
> Because I think that what it can do is simply to do the ->poll() calls
> outside the iocb locks, and then just attach the poll table to the
> kioctx afterwards.
We could do that on the submit side fairly easily. The problem
is really the completion side, where I'd much avoid introducing a
spurious context switch. Right now even with a NULL qproc we can't
guarantee any of that. So we'll need to schedule out to a workqueue,
and then from that schedule the potential multiple NULL qproc calls,
which might actually block elsewhere even if __pollwait is never called.
> This whole "poll must not block" is a complete red herring. It doesn't
> come from any other requirements than BAD AIO GARBAGE CODE.
I comes from the fact to avoid a totally pointless context switch.
aio code itself works just fine called from a workqueue, we have
exatly that case when file system do non-trivial operations in their
end_io handler.
^ permalink raw reply
* Re: [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer
From: Christoph Hellwig @ 2018-06-29 13:29 UTC (permalink / raw)
To: Al Viro
Cc: Linus Torvalds, Christoph Hellwig, linux-fsdevel,
Network Development, LKP
In-Reply-To: <20180628213027.GK30522@ZenIV.linux.org.uk>
On Thu, Jun 28, 2018 at 10:30:27PM +0100, Al Viro wrote:
> > Because I think that what it can do is simply to do the ->poll() calls
> > outside the iocb locks, and then just attach the poll table to the
> > kioctx afterwards.
>
> I'd do a bit more - embed the first poll_table_entry into poll iocb itself,
> so that the instances that use only one queue wouldn't need any allocations
> at all.
No need for poll_table_entry, we just need a wait_queue_head.
poll_table_entry is an select.c internal (except for two nasty driver) -
neither epoll nor most in-kernel callers use it.
^ permalink raw reply
* Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: David Miller @ 2018-06-29 13:32 UTC (permalink / raw)
To: dsahern
Cc: jiri, jhs, xiyou.wangcong, g, netdev, jakub.kicinski,
simon.horman, john.hurley, mlxsw, sridhar.samudrala
In-Reply-To: <95856cb5-62bc-5b1d-6469-bb72c886891a@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Fri, 29 Jun 2018 06:54:36 -0600
> The resolution of the syntax affect the uapi changes proposed. You are
> wanting to add new RTM commands which suggests new objects. If a
> template is an attribute of an existing object then the netlink API
> should indicate that.
+1
^ permalink raw reply
* Re: Diagnosing network module for missing link establishment (cxgb3, Chelsio T320)
From: Andrew Lunn @ 2018-06-29 13:33 UTC (permalink / raw)
To: U.Mutlu; +Cc: netdev
In-Reply-To: <5B356C76.3050809@mutluit.com>
> Kernel: 3.16.0-4-amd64 (Debian v8 stock kernel)
This is very old. Please at least try the current version of Debian,
"Stretch".
Andrew
^ permalink raw reply
* [PATCH 1/2] net: handle NULL ->poll gracefully
From: Christoph Hellwig @ 2018-06-29 13:37 UTC (permalink / raw)
To: torvalds; +Cc: viro, netdev, linux-fsdevel
In-Reply-To: <20180629133725.17606-1-hch@lst.de>
The big aio poll revert broke various network protocols that don't
implement ->poll as a patch in the aio poll serie removed sock_no_poll
and made the common code handle this case.
Fixes: a11e1d43 ("Revert changes to convert to ->poll_mask() and aio IOCB_CMD_POLL")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
net/socket.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/socket.c b/net/socket.c
index a564c6ed19d5..85633622c94d 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1133,6 +1133,8 @@ static __poll_t sock_poll(struct file *file, poll_table *wait)
__poll_t events = poll_requested_events(wait);
sock_poll_busy_loop(sock, events);
+ if (!sock->ops->poll)
+ return 0;
return sock->ops->poll(file, sock, wait) | sock_poll_busy_flag(sock);
}
--
2.18.0
^ permalink raw reply related
* Fixes for the AIO poll revert
From: Christoph Hellwig @ 2018-06-29 13:37 UTC (permalink / raw)
To: torvalds; +Cc: viro, netdev, linux-fsdevel
Two fixup for incorrectly reverted bits.
^ permalink raw reply
* [PATCH 2/2] aio: mark __aio_sigset::sigmask const
From: Christoph Hellwig @ 2018-06-29 13:37 UTC (permalink / raw)
To: torvalds; +Cc: viro, netdev, linux-fsdevel, Avi Kivity
In-Reply-To: <20180629133725.17606-1-hch@lst.de>
From: Avi Kivity <avi@scylladb.com>
io_pgetevents() will not change the signal mask. Mark it const
to make it clear and to reduce the need for casts in user code.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Avi Kivity <avi@scylladb.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
[hch: reapply the patch that got incorrectly reverted]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/uapi/linux/aio_abi.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
index d4e768d55d14..3c5038b587ba 100644
--- a/include/uapi/linux/aio_abi.h
+++ b/include/uapi/linux/aio_abi.h
@@ -111,7 +111,7 @@ struct iocb {
#undef IFLITTLE
struct __aio_sigset {
- sigset_t __user *sigmask;
+ const sigset_t __user *sigmask;
size_t sigsetsize;
};
--
2.18.0
^ permalink raw reply related
* Re: [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer
From: Linus Torvalds @ 2018-06-29 13:40 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Al Viro, linux-fsdevel, Network Development, LKP
In-Reply-To: <20180629132924.GB28510@lst.de>
On Fri, Jun 29, 2018 at 6:29 AM Christoph Hellwig <hch@lst.de> wrote:
> No need for poll_table_entry, we just need a wait_queue_head.
> poll_table_entry is an select.c internal (except for two nasty driver) -
> neither epoll nor most in-kernel callers use it.
Well, you need the poll_table for the "poll_wait()", and you would
need to be able to have multiple wait-queues even though you only have
one file you're waiting for.
But yes, it doesn't really need to be the full complex poll_table_entry model.
Linus
^ permalink raw reply
* Re: Link modes representation in phylib
From: Andrew Lunn @ 2018-06-29 13:43 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, Russell King - ARM Linux, Florian Fainelli, netdev,
Antoine Tenart, thomas.petazzoni@bootlin.com, Gregory CLEMENT,
Miquel Raynal
In-Reply-To: <20180629152613.43fcba8d@bootlin.com>
> Thanks for the suggestion. I see how this can be done with
> phydrv->supported and phydev->lp_advertising, however I'm not sure how
> we should deal with the fact that ethernet drivers directly access
> fields such as "advertising" or "supported".
Hi Maxime
I started cleaning up some of the MAC drivers. Take a look at
https://github.com/lunn/linux.git v4.18-rc1-net-next-phy-cleanup
That moved a lot of the MAC code into helpers, making it easier to
update. It might make sense to add a couple of more helpers, for what
remains.
Andrew
^ permalink raw reply
* Re: [bpf PATCH v4 3/4] bpf: sockhash fix omitted bucket lock in sock_close
From: Daniel Borkmann @ 2018-06-29 13:45 UTC (permalink / raw)
To: John Fastabend, ast, kafai; +Cc: netdev
In-Reply-To: <20180625153417.3057.19275.stgit@john-Precision-Tower-5810>
On 06/25/2018 05:34 PM, John Fastabend wrote:
[...]
> @@ -2302,9 +2347,12 @@ static int sock_hash_ctx_update_elem(struct bpf_sock_ops_kern *skops,
> goto bucket_err;
> }
>
> - e->hash_link = l_new;
> - e->htab = container_of(map, struct bpf_htab, map);
> + rcu_assign_pointer(e->hash_link, l_new);
> + rcu_assign_pointer(e->htab,
> + container_of(map, struct bpf_htab, map));
> + write_lock_bh(&sock->sk_callback_lock);
> list_add_tail(&e->list, &psock->maps);
> + write_unlock_bh(&sock->sk_callback_lock);
>
> /* add new element to the head of the list, so that
> * concurrent search will find it before old elem
> @@ -2314,8 +2362,10 @@ static int sock_hash_ctx_update_elem(struct bpf_sock_ops_kern *skops,
> psock = smap_psock_sk(l_old->sk);
>
> hlist_del_rcu(&l_old->hash_node);
> + write_lock_bh(&l_old->sk->sk_callback_lock);
> smap_list_hash_remove(psock, l_old);
> smap_release_sock(psock, l_old->sk);
> + write_unlock_bh(&l_old->sk->sk_callback_lock);
> free_htab_elem(htab, l_old);
> }
> raw_spin_unlock_bh(&b->lock);
Rather a question for clarification that came up while staring at this part
in sock_hash_ctx_update_elem():
In the 'err' label, wouldn't we also need the sk_callback_lock given we access
a potential psock, and modify its state (refcnt) would this be needed as well,
or are we good here since we're under RCU context anyway? Hmm, if latter is
indeed the case, then I'm wondering why we would need the above /sock/'s
sk_callback_lock for modifying list handling inside the psock and couldn't use
some locking that is only in scope of the actual struct smap_psock?
My other question is, if someone calls the update helper with BPF_NOEXIST which
we seem to support with bailing out via -EEXIST, and if there's currently a
psock attached already to the sock, then we drop this one off from the socket?
Is that correct / expected?
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH 1/2] net: handle NULL ->poll gracefully
From: Linus Torvalds @ 2018-06-29 13:46 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Al Viro, Network Development, linux-fsdevel
In-Reply-To: <20180629133725.17606-2-hch@lst.de>
On Fri, Jun 29, 2018 at 6:37 AM Christoph Hellwig <hch@lst.de> wrote:
>
> The big aio poll revert broke various network protocols that don't
> implement ->poll as a patch in the aio poll serie removed sock_no_poll
> and made the common code handle this case.
Hmm. I just did the revert of commit 984652dd8b1f ("net: remove
sock_no_poll") in my tree.
But I haven't pushed it out, and your patch is certainly smaller/nicer
than the revert.
That said, the revert does have the advantage of avoiding a
conditional in the network poll() function (at the cost of a more
expensive indirect call for when sock_no_poll actually triggers, but
only "odd" network protocols have that).
Hmm. I'll have to think about it.
Linus
^ permalink raw reply
* Re: [PATCH 1/3] ath10k: sdio: use same endpoint id for all packets in a bundle
From: Kalle Valo @ 2018-06-29 13:51 UTC (permalink / raw)
To: Niklas Cassel
Cc: alagusankar, netdev, linux-wireless, linux-kernel, ath10k,
David S. Miller
In-Reply-To: <20180629131141.GA26446@centauri.lan>
Niklas Cassel <niklas.cassel@linaro.org> writes:
> On Fri, Jun 29, 2018 at 02:53:44PM +0300, Kalle Valo wrote:
>> Kalle Valo <kvalo@codeaurora.org> writes:
>>
>> > Niklas Cassel <niklas.cassel@linaro.org> writes:
>> >
>> >> All packets in a bundle should use the same endpoint id as the
>> >> first lookahead.
>> >>
>> >> This matches how things are done is ath6kl, however,
>> >> this patch can theoretically handle several bundles
>> >> in ath10k_sdio_mbox_rx_process_packets().
>> >>
>> >> Without this patch we get lots of errors about invalid endpoint id:
>> >>
>> >> ath10k_sdio mmc2:0001:1: invalid endpoint in look-ahead: 224
>> >> ath10k_sdio mmc2:0001:1: failed to get pending recv messages: -12
>> >> ath10k_sdio mmc2:0001:1: failed to process pending SDIO interrupts: -12
>> >>
>> >> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
>> >> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
>> >
>> > You have Alagu's s-o-b first so that implies Alagu is the author. So
>> > should I add From header for Alagu and add you (Niklas) as
>> > Co-Developed-by? Or vice versa?
>
> Hello Kalle,
>
> It is not always obvious how the combination of git-author
> and Co-Developed-by should be:
> http://lkml.iu.edu/hypermail/linux/kernel/1801.2/00988.html
Yeah, that's true.
> Alagu deserves most credit, since I have simply taken parts
> of a very big patch and split it into smaller pieces.
> I tried to do the right thing by having his Signed-off-by first.
>
> Let's go with your suggestion and add a From: header with
> Alagu's email, and a Co-Developed-by tag with my email.
> (Note that both Signed-off-bys are still needed according to
> submitting-patches.rst)
Ok.
> Do you want me to resend the patches with these two lines
> added, or can you fix them up manually?
No need, I did that already in the pending branch. Please check:
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6b23bd067990df1cc1e9a4c28327393a7a3ba718
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=bf83a5c572e0737e59e026c989b190b4e52d2ef4
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=218f924d80b76520a4e60eead5e89f3ebd2e86c1
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 1/2] net: handle NULL ->poll gracefully
From: Linus Torvalds @ 2018-06-29 13:53 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Al Viro, Network Development, linux-fsdevel
In-Reply-To: <CA+55aFwRmQCSYF_yHUSZDBjTnmP=2n5rSRP-6BWzNA=6h_+T_w@mail.gmail.com>
On Fri, Jun 29, 2018 at 6:46 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Hmm. I'll have to think about it.
I took yours over the revert. It's just smaller and nicer, and the
conditional should predict fine for any case where it might matter.
Linus
^ permalink raw reply
* [PATCH net-next] strparser: Call skb_unclone conditionally
From: Vakul Garg @ 2018-06-29 19:15 UTC (permalink / raw)
To: davem; +Cc: netdev, tom, borisp, ebiggers, davejwatson, doronrk, Vakul Garg
Calling skb_unclone() is expensive as it triggers a memcpy operation.
Instead of calling skb_unclone() unconditionally, call it only when skb
has a shared frag_list. This improves tls rx throughout significantly.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
Suggested-by: Boris Pismenny <borisp@mellanox.com>
---
net/strparser/strparser.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
index 373836615c57..4f40a90ca016 100644
--- a/net/strparser/strparser.c
+++ b/net/strparser/strparser.c
@@ -155,11 +155,13 @@ static int __strp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb,
/* We are going to append to the frags_list of head.
* Need to unshare the frag_list.
*/
- err = skb_unclone(head, GFP_ATOMIC);
- if (err) {
- STRP_STATS_INCR(strp->stats.mem_fail);
- desc->error = err;
- return 0;
+ if (skb_has_frag_list(head)) {
+ err = skb_unclone(head, GFP_ATOMIC);
+ if (err) {
+ STRP_STATS_INCR(strp->stats.mem_fail);
+ desc->error = err;
+ return 0;
+ }
}
if (unlikely(skb_shinfo(head)->frag_list)) {
@@ -216,14 +218,16 @@ static int __strp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb,
memset(stm, 0, sizeof(*stm));
stm->strp.offset = orig_offset + eaten;
} else {
- /* Unclone since we may be appending to an skb that we
+ /* Unclone if we are appending to an skb that we
* already share a frag_list with.
*/
- err = skb_unclone(skb, GFP_ATOMIC);
- if (err) {
- STRP_STATS_INCR(strp->stats.mem_fail);
- desc->error = err;
- break;
+ if (skb_has_frag_list(skb)) {
+ err = skb_unclone(skb, GFP_ATOMIC);
+ if (err) {
+ STRP_STATS_INCR(strp->stats.mem_fail);
+ desc->error = err;
+ break;
+ }
}
stm = _strp_msg(head);
--
2.13.6
^ permalink raw reply related
* Re: [PATCH 1/3] ath10k: sdio: use same endpoint id for all packets in a bundle
From: Niklas Cassel @ 2018-06-29 13:56 UTC (permalink / raw)
To: Kalle Valo
Cc: alagusankar, netdev, linux-wireless, linux-kernel, ath10k,
David S. Miller
In-Reply-To: <87r2kpem4o.fsf@codeaurora.org>
On Fri, Jun 29, 2018 at 04:51:51PM +0300, Kalle Valo wrote:
> Niklas Cassel <niklas.cassel@linaro.org> writes:
>
> > On Fri, Jun 29, 2018 at 02:53:44PM +0300, Kalle Valo wrote:
> >> Kalle Valo <kvalo@codeaurora.org> writes:
> >>
> >> > Niklas Cassel <niklas.cassel@linaro.org> writes:
> >> >
> >> >> All packets in a bundle should use the same endpoint id as the
> >> >> first lookahead.
> >> >>
> >> >> This matches how things are done is ath6kl, however,
> >> >> this patch can theoretically handle several bundles
> >> >> in ath10k_sdio_mbox_rx_process_packets().
> >> >>
> >> >> Without this patch we get lots of errors about invalid endpoint id:
> >> >>
> >> >> ath10k_sdio mmc2:0001:1: invalid endpoint in look-ahead: 224
> >> >> ath10k_sdio mmc2:0001:1: failed to get pending recv messages: -12
> >> >> ath10k_sdio mmc2:0001:1: failed to process pending SDIO interrupts: -12
> >> >>
> >> >> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
> >> >> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> >> >
> >> > You have Alagu's s-o-b first so that implies Alagu is the author. So
> >> > should I add From header for Alagu and add you (Niklas) as
> >> > Co-Developed-by? Or vice versa?
> >
> > Hello Kalle,
> >
> > It is not always obvious how the combination of git-author
> > and Co-Developed-by should be:
> > http://lkml.iu.edu/hypermail/linux/kernel/1801.2/00988.html
>
> Yeah, that's true.
>
> > Alagu deserves most credit, since I have simply taken parts
> > of a very big patch and split it into smaller pieces.
> > I tried to do the right thing by having his Signed-off-by first.
> >
> > Let's go with your suggestion and add a From: header with
> > Alagu's email, and a Co-Developed-by tag with my email.
> > (Note that both Signed-off-bys are still needed according to
> > submitting-patches.rst)
>
> Ok.
>
> > Do you want me to resend the patches with these two lines
> > added, or can you fix them up manually?
>
> No need, I did that already in the pending branch. Please check:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6b23bd067990df1cc1e9a4c28327393a7a3ba718
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=bf83a5c572e0737e59e026c989b190b4e52d2ef4
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=218f924d80b76520a4e60eead5e89f3ebd2e86c1
Hello Kalle
It looks good :)
Regards,
Niklas
>
> --
> Kalle Valo
^ permalink raw reply
* Re: [net-next 2/3] tipc: implement socket diagnostics for AF_TIPC
From: Eric Dumazet @ 2018-06-29 13:57 UTC (permalink / raw)
To: GhantaKrishnamurthy MohanKrishna, tipc-discussion, jon.maloy,
maloy, ying.xue, netdev, davem
Cc: Parthasarathy Bhuvaragan
In-Reply-To: <1521639465-3169-3-git-send-email-mohan.krishna.ghanta.krishnamurthy@ericsson.com>
On 03/21/2018 06:37 AM, GhantaKrishnamurthy MohanKrishna wrote:
> This commit adds socket diagnostics capability for AF_TIPC in netlink
> family NETLINK_SOCK_DIAG in a new kernel module (diag.ko).
...
> +static u64 __tipc_diag_gen_cookie(struct sock *sk)
> +{
> + u32 res[2];
> +
> + sock_diag_save_cookie(sk, res);
> + return *((u64 *)res);
> +}
I am pretty sure some combination of compiler / arch could be not happy
with this assumption that res[] is aligned to allow an u64 to be read like that.
^ 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