* Re: [PATCH v2 7/7] batman-adv: ELP - add throughput meter test duration attribute
From: Sven Eckelmann @ 2018-08-04 9:02 UTC (permalink / raw)
To: Antonio Quartulli
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
The list for a Better Approach To Mobile Ad-hoc Networking,
Jiri Pirko, Marek Lindner
In-Reply-To: <314bf0ac-4c10-da7a-d527-45afe92423fa-2CpIooy/SPIKlTDg6p0iyA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]
On Samstag, 4. August 2018 10:41:09 CEST Antonio Quartulli wrote:
[...]
> >> Documentation/ABI/testing/sysfs-class-net-batman-adv | 7 +++++++
> >
> > Please discuss this with Jiri [1].
> >
>
> After re-reading Jiri's points I can't really understand why we should
> now switch to netlink. I think all our sysfs knobs are used to inject
> settings *to* userspace, therefore his point 1) does not really apply to
> us. Point 2) is a bit generic and does not really explain why we should
> *switch*.
Wouldn't it have been better when Jiri would also see your reply? Now he isn't
even aware of your criticism.
> This said, I'd rather keep this patch as it is and possibly discuss the
> matter when sending this code to netdev for merging.
>
> I've discussed this with Marek too and he is fine with this approach.
Interesting, I don't see it this way. This patch [0] is for netdev and we have
the statement that "usage of sysfs in netdev subsystem is frowned upon". Now
you want that we ignore that and than maybe Simon have to deal with the
fallout when he forwards it to David?
Kind regards,
Sven
[0] https://patchwork.open-mesh.org/patch/17372/
> > [1] https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2018-May/017814.html
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 4.4 106/124] netlink: Do not subscribe to non-existent groups
From: Greg Kroah-Hartman @ 2018-08-04 9:01 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, David S. Miller, Herbert Xu,
Steffen Klassert, netdev, Dmitry Safonov
In-Reply-To: <20180804082702.434482435@linuxfoundation.org>
4.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Dmitry Safonov <dima@arista.com>
[ Upstream commit 7acf9d4237c46894e0fa0492dd96314a41742e84 ]
Make ABI more strict about subscribing to group > ngroups.
Code doesn't check for that and it looks bogus.
(one can subscribe to non-existing group)
Still, it's possible to bind() to all possible groups with (-1)
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Dmitry Safonov <dima@arista.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/netlink/af_netlink.c | 1 +
1 file changed, 1 insertion(+)
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -983,6 +983,7 @@ static int netlink_bind(struct socket *s
if (err)
return err;
}
+ groups &= (1UL << nlk->ngroups) - 1;
bound = nlk->bound;
if (bound) {
^ permalink raw reply
* [PATCH 4.9 09/32] netlink: Do not subscribe to non-existent groups
From: Greg Kroah-Hartman @ 2018-08-04 9:00 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, David S. Miller, Herbert Xu,
Steffen Klassert, netdev, Dmitry Safonov
In-Reply-To: <20180804082649.908295462@linuxfoundation.org>
4.9-stable review patch. If anyone has any objections, please let me know.
------------------
From: Dmitry Safonov <dima@arista.com>
[ Upstream commit 7acf9d4237c46894e0fa0492dd96314a41742e84 ]
Make ABI more strict about subscribing to group > ngroups.
Code doesn't check for that and it looks bogus.
(one can subscribe to non-existing group)
Still, it's possible to bind() to all possible groups with (-1)
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Dmitry Safonov <dima@arista.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/netlink/af_netlink.c | 1 +
1 file changed, 1 insertion(+)
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -983,6 +983,7 @@ static int netlink_bind(struct socket *s
if (err)
return err;
}
+ groups &= (1UL << nlk->ngroups) - 1;
bound = nlk->bound;
if (bound) {
^ permalink raw reply
* [PATCH bpf-next] nfp: bpf: xdp_adjust_tail support
From: Jakub Kicinski @ 2018-08-04 5:06 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, oss-drivers, Jakub Kicinski
Add support for adjust_tail. There are no FW changes needed but add
a FW capability just in case there would be any issue with previously
released FW, or we will have to change the ABI in the future.
The helper is trivial and shouldn't be used too often so just inline
the body of the function. We add the delta to locally maintained
packet length register and check for overflow, since add of negative
value must overflow if result is positive. Note that if delta of 0
would be allowed in the kernel this trick stops working and we need
one more instruction to compare lengths before and after the change.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/fw.h | 1 +
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 47 +++++++++++++++++++
drivers/net/ethernet/netronome/nfp/bpf/main.c | 13 +++++
drivers/net/ethernet/netronome/nfp/bpf/main.h | 2 +
.../net/ethernet/netronome/nfp/bpf/verifier.c | 7 +++
drivers/net/ethernet/netronome/nfp/nfp_asm.h | 1 +
6 files changed, 71 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/fw.h b/drivers/net/ethernet/netronome/nfp/bpf/fw.h
index 4c7972e3db63..e4f9b7ec8528 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/fw.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/fw.h
@@ -51,6 +51,7 @@ enum bpf_cap_tlv_type {
NFP_BPF_CAP_TYPE_MAPS = 3,
NFP_BPF_CAP_TYPE_RANDOM = 4,
NFP_BPF_CAP_TYPE_QUEUE_SELECT = 5,
+ NFP_BPF_CAP_TYPE_ADJUST_TAIL = 6,
};
struct nfp_bpf_cap_tlv_func {
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index 3c22d27de9da..eff57f7d056a 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -1642,6 +1642,51 @@ static int adjust_head(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
return 0;
}
+static int adjust_tail(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
+{
+ u32 ret_einval, end;
+ swreg plen, delta;
+
+ BUILD_BUG_ON(plen_reg(nfp_prog) != reg_b(STATIC_REG_PKT_LEN));
+
+ plen = imm_a(nfp_prog);
+ delta = reg_a(2 * 2);
+
+ ret_einval = nfp_prog_current_offset(nfp_prog) + 9;
+ end = nfp_prog_current_offset(nfp_prog) + 11;
+
+ /* Calculate resulting length */
+ emit_alu(nfp_prog, plen, plen_reg(nfp_prog), ALU_OP_ADD, delta);
+ /* delta == 0 is not allowed by the kernel, add must overflow to make
+ * length smaller.
+ */
+ emit_br(nfp_prog, BR_BCC, ret_einval, 0);
+
+ /* if (new_len < 14) then -EINVAL */
+ emit_alu(nfp_prog, reg_none(), plen, ALU_OP_SUB, reg_imm(ETH_HLEN));
+ emit_br(nfp_prog, BR_BMI, ret_einval, 0);
+
+ emit_alu(nfp_prog, plen_reg(nfp_prog),
+ plen_reg(nfp_prog), ALU_OP_ADD, delta);
+ emit_alu(nfp_prog, pv_len(nfp_prog),
+ pv_len(nfp_prog), ALU_OP_ADD, delta);
+
+ emit_br(nfp_prog, BR_UNC, end, 2);
+ wrp_immed(nfp_prog, reg_both(0), 0);
+ wrp_immed(nfp_prog, reg_both(1), 0);
+
+ if (!nfp_prog_confirm_current_offset(nfp_prog, ret_einval))
+ return -EINVAL;
+
+ wrp_immed(nfp_prog, reg_both(0), -22);
+ wrp_immed(nfp_prog, reg_both(1), ~0);
+
+ if (!nfp_prog_confirm_current_offset(nfp_prog, end))
+ return -EINVAL;
+
+ return 0;
+}
+
static int
map_call_stack_common(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
@@ -3041,6 +3086,8 @@ static int call(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
switch (meta->insn.imm) {
case BPF_FUNC_xdp_adjust_head:
return adjust_head(nfp_prog, meta);
+ case BPF_FUNC_xdp_adjust_tail:
+ return adjust_tail(nfp_prog, meta);
case BPF_FUNC_map_lookup_elem:
case BPF_FUNC_map_update_elem:
case BPF_FUNC_map_delete_elem:
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
index cce1d2945a32..970af07f4656 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
@@ -334,6 +334,14 @@ nfp_bpf_parse_cap_qsel(struct nfp_app_bpf *bpf, void __iomem *value, u32 length)
return 0;
}
+static int
+nfp_bpf_parse_cap_adjust_tail(struct nfp_app_bpf *bpf, void __iomem *value,
+ u32 length)
+{
+ bpf->adjust_tail = true;
+ return 0;
+}
+
static int nfp_bpf_parse_capabilities(struct nfp_app *app)
{
struct nfp_cpp *cpp = app->pf->cpp;
@@ -380,6 +388,11 @@ static int nfp_bpf_parse_capabilities(struct nfp_app *app)
if (nfp_bpf_parse_cap_qsel(app->priv, value, length))
goto err_release_free;
break;
+ case NFP_BPF_CAP_TYPE_ADJUST_TAIL:
+ if (nfp_bpf_parse_cap_adjust_tail(app->priv, value,
+ length))
+ goto err_release_free;
+ break;
default:
nfp_dbg(cpp, "unknown BPF capability: %d\n", type);
break;
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index 57573bfa8c03..dbd00982fd2b 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -150,6 +150,7 @@ enum pkt_vec {
*
* @pseudo_random: FW initialized the pseudo-random machinery (CSRs)
* @queue_select: BPF can set the RX queue ID in packet vector
+ * @adjust_tail: BPF can simply trunc packet size for adjust tail
*/
struct nfp_app_bpf {
struct nfp_app *app;
@@ -195,6 +196,7 @@ struct nfp_app_bpf {
bool pseudo_random;
bool queue_select;
+ bool adjust_tail;
};
enum nfp_bpf_map_use {
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
index 49ba0d645d36..a6e9248669e1 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
@@ -178,6 +178,13 @@ nfp_bpf_check_call(struct nfp_prog *nfp_prog, struct bpf_verifier_env *env,
nfp_record_adjust_head(bpf, nfp_prog, meta, reg2);
break;
+ case BPF_FUNC_xdp_adjust_tail:
+ if (!bpf->adjust_tail) {
+ pr_vlog(env, "adjust_tail not supported by FW\n");
+ return -EOPNOTSUPP;
+ }
+ break;
+
case BPF_FUNC_map_lookup_elem:
if (!nfp_bpf_map_call_ok("map_lookup", env, meta,
bpf->helpers.map_lookup, reg1) ||
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_asm.h b/drivers/net/ethernet/netronome/nfp/nfp_asm.h
index cdc4e065f6f5..fad0e62a910c 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_asm.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_asm.h
@@ -93,6 +93,7 @@ enum br_mask {
BR_BNE = 0x01,
BR_BMI = 0x02,
BR_BHS = 0x04,
+ BR_BCC = 0x05,
BR_BLO = 0x05,
BR_BGE = 0x08,
BR_BLT = 0x09,
--
2.17.1
^ permalink raw reply related
* Re: [pull request][net-next 00/10] Mellanox, mlx5 and devlink updates 2018-07-31
From: Jakub Kicinski @ 2018-08-04 4:59 UTC (permalink / raw)
To: Ido Schimmel
Cc: Eran Ben Elisha, David Miller, saeedm, netdev, jiri,
alexander.duyck, helgaas
In-Reply-To: <20180803164150.GA21806@splinter.mtl.com>
On Fri, 3 Aug 2018 19:41:50 +0300, Ido Schimmel wrote:
> On Thu, Aug 02, 2018 at 03:53:15PM -0700, Jakub Kicinski wrote:
> > No one is requesting full RED offload here.. if someone sets the
> > parameters you can't support you simply won't offload them. And ignore
> > the parameters which only make sense in software terms. Look at the
> > docs for mlxsw:
> >
> > https://github.com/Mellanox/mlxsw/wiki/Queues-Management#offloading-red
> >
> > It says "not offloaded" in a number of places.
> >
> ...
> > It's generally preferable to implement a subset of exiting well defined
> > API than create vendor knobs, hence hardly a misuse.
>
> Sorry for derailing the discussion, but you mentioned some points that
> have been bothering me for a while.
>
> I think we didn't do a very good job with buffer management and this is
> exactly why you see some parameters marked as "not offloaded". Take the
> "limit" (queue size) for example. It's configured via devlink-sb, by
> setting a quota on the number of bytes that can be queued for the port
> and TC (queue) that RED manages. See:
>
> https://github.com/Mellanox/mlxsw/wiki/Quality-of-Service#pool-binding
FWIW I was implementing a very similar thing for the NFP a while back.
devlink-sb to configure per-port limits and RED offload. I believe we
have some more qdisc offloads but out-of-tree/for appliances.
"Switchdev mode" + qdisc offloads work quite well. For RED I think
we also don't offload the limit.
> It would have been much better and user friendly to not ignore this
> parameter and have users configure the limit using existing interfaces
> (tc), instead of creating a discrepancy between the software and
> hardware data paths by configuring the hardware directly via devlink-sb.
>
> I believe devlink-sb is mainly the result of Linux's short comings in
> this area and our lack of perspective back then. While the qdisc layer
> (Linux's shared buffers) works for end hosts, it requires enhancements
> (mainly on ingress) for switches (physical/virtual) that forward
> packets.
I could definitely agree with you. But there is another way to look at
this. Memory in ASICs is fundamentally more precious. If the problem
was never solved for Linux (placing constraints on the number of
packets in the system by ingress port) maybe it's just not important
for software stacks? Qdiscs are focused on egress. Perhaps a better
software equivalent to Shared Buffers would be Jesper's Buffer Pools?
With Buffer Pools the concern that a pre-configured and pinned pool of
DMA-mapped pages will start growing and eat all host's memory is more
real. That to me that's closer. If we develop XDP-based fastpaths
with DMA pools shared between devices - that's much more like an ASIC's
SB.
In my view we don't offload the limit not because we configure it via
an different API, but because the limit assumes there is abundance of
memory and queue has to be capped. Limit expresses how much queue
build up is okay, while SB config is strictly a resource quota. In
practice the quota is always a lot lower than user's desired limit so
we don't even bother with the limit.
> For example, switches (I'm familiar with Mellanox ASICs, but I assume
> the concept is similar in other ASICs) have ingress buffers where
> packets are stored while going through the pipeline. Once out of the
> pipeline you know from which port and queue the packet should egress. In
> case you have both lossless and lossy traffic in your network you
> probably want to classify it into different ingress buffers and mark the
> buffers where the lossless traffic is stored as such, so that PFC frames
> would be emitted above a certain threshold.
>
> This is currently configured using dcbnl, but it lacks a software model
> which means that packets that are forwarded by the kernel don't get the
> same treatment (e.g., skb priority isn't set). It also means that when
> you want to limit the number of packets that are queued *from* a certain
> port and ingress buffer you resort to tools such as devlink-sb that end
> up colliding with existing tools (tc).
Extending DCB further into the kernel on ingress does not seem
impossible. Maybe the AVB/industrial folks will tackle that at some
point?
> I was thinking (not too much...) about modelling the above using ingress
> qdiscs. They don't do any queueing, but more of accounting. Once the
> egress qdisc dequeues the packet, you give credit back to the ingress
> qdisc from which the packet came from. I believe that modelling these
> buffers using the qdisc layer is the right abstraction.
Interesting. My concern would be mapping the packet back to ingress
port to free the right qdisc credit. MM direction, like Buffer Pools,
seem more viable to a layman like me. But ingress qdiscs sound worth
exploring.
> Would appreciate hearing your thoughts on the above.
Thanks a lot for your response, you've certainly given me things to
think about over the weekend :) A lot of cool things we can build if
we keep moving forward :)
^ permalink raw reply
* Re: [net-next, v6, 6/7] net-sysfs: Add interface for Rx queue(s) map per Tx queue
From: Andrei Vagin @ 2018-08-04 4:30 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Nambiar, Amritha, netdev, davem, alexander.h.duyck,
willemdebruijn.kernel, sridhar.samudrala, alexander.duyck,
edumazet, hannes, tom, tom, jasowang, gaowanlong, virtualization,
rostedt
In-Reply-To: <20180803221212-mutt-send-email-mst@kernel.org>
On Fri, Aug 03, 2018 at 10:12:53PM +0300, Michael S. Tsirkin wrote:
> On Fri, Aug 03, 2018 at 12:06:51PM -0700, Andrei Vagin wrote:
> > On Fri, Aug 03, 2018 at 12:08:05AM +0300, Michael S. Tsirkin wrote:
> > > On Thu, Aug 02, 2018 at 02:04:12PM -0700, Nambiar, Amritha wrote:
> > > > On 8/1/2018 5:11 PM, Andrei Vagin wrote:
> > > > > On Tue, Jul 10, 2018 at 07:28:49PM -0700, Nambiar, Amritha wrote:
> > > > >> With this patch series, I introduced static_key for XPS maps
> > > > >> (xps_needed), so static_key_slow_inc() is used to switch branches. The
> > > > >> definition of static_key_slow_inc() has cpus_read_lock in place. In the
> > > > >> virtio_net driver, XPS queues are initialized after setting the
> > > > >> queue:cpu affinity in virtnet_set_affinity() which is already protected
> > > > >> within cpus_read_lock. Hence, the warning here trying to acquire
> > > > >> cpus_read_lock when it is already held.
> > > > >>
> > > > >> A quick fix for this would be to just extract netif_set_xps_queue() out
> > > > >> of the lock by simply wrapping it with another put/get_online_cpus
> > > > >> (unlock right before and hold lock right after).
> > > > >
> > > > > virtnet_set_affinity() is called from virtnet_cpu_online(), which is
> > > > > called under cpus_read_lock too.
> > > > >
> > > > > It looks like now we can't call netif_set_xps_queue() from cpu hotplug
> > > > > callbacks.
> > > > >
> > > > > I can suggest a very straightforward fix for this problem. The patch is
> > > > > attached.
> > > > >
> > > >
> > > > Thanks for looking into this. I was thinking of fixing this in the
> > > > virtio_net driver by moving the XPS initialization (and have a new
> > > > get_affinity utility) in the ndo_open (so it is together with other tx
> > > > preparation) instead of probe. Your patch solves this in general for
> > > > setting up cpu hotplug callbacks which is under cpus_read_lock.
> > >
> > >
> > > I like this too. Could you repost in a standard way
> > > (inline, with your signoff etc) so we can ack this for
> > > net-next?
> >
> > When I was testing this patch, I got the following kasan warning. Michael,
> > could you take a look at it. Maybe you will understand what was going wrong there.
> >
> > https://api.travis-ci.org/v3/job/410701353/log.txt
> >
> > [ 7.275033] ==================================================================
> > [ 7.275226] BUG: KASAN: slab-out-of-bounds in virtnet_poll+0xaa1/0xd00
> > [ 7.275359] Read of size 8 at addr ffff8801d444a000 by task ip/370
> > [ 7.275488]
> > [ 7.275610] CPU: 1 PID: 370 Comm: ip Not tainted 4.18.0-rc6+ #1
> > [ 7.275613] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> > [ 7.275616] Call Trace:
> > [ 7.275621] <IRQ>
> > [ 7.275630] dump_stack+0x71/0xab
> > [ 7.275640] print_address_description+0x6a/0x270
> > [ 7.275648] kasan_report+0x258/0x380
> > [ 7.275653] ? virtnet_poll+0xaa1/0xd00
> > [ 7.275661] virtnet_poll+0xaa1/0xd00
> > [ 7.275680] ? receive_buf+0x5920/0x5920
> > [ 7.275689] ? do_raw_spin_unlock+0x54/0x220
> > [ 7.275699] ? find_held_lock+0x32/0x1c0
> > [ 7.275710] ? rcu_process_callbacks+0xa60/0xd20
> > [ 7.275736] net_rx_action+0x2ee/0xad0
> > [ 7.275748] ? rcu_note_context_switch+0x320/0x320
> > [ 7.275754] ? napi_complete_done+0x300/0x300
> > [ 7.275763] ? native_apic_msr_write+0x27/0x30
> > [ 7.275768] ? lapic_next_event+0x5b/0x90
> > [ 7.275775] ? clockevents_program_event+0x21d/0x2f0
> > [ 7.275791] __do_softirq+0x19a/0x623
> > [ 7.275807] do_softirq_own_stack+0x2a/0x40
> > [ 7.275811] </IRQ>
> > [ 7.275818] do_softirq.part.18+0x6a/0x80
> > [ 7.275825] __local_bh_enable_ip+0x49/0x50
> > [ 7.275829] virtnet_open+0x129/0x440
> > [ 7.275841] __dev_open+0x189/0x2c0
> > [ 7.275848] ? dev_set_rx_mode+0x30/0x30
> > [ 7.275857] ? do_raw_spin_unlock+0x54/0x220
> > [ 7.275866] __dev_change_flags+0x3a9/0x4f0
> > [ 7.275873] ? dev_set_allmulti+0x10/0x10
> > [ 7.275889] dev_change_flags+0x7a/0x150
> > [ 7.275900] do_setlink+0x9fe/0x2e40
> > [ 7.275910] ? deref_stack_reg+0xad/0xe0
> > [ 7.275917] ? __read_once_size_nocheck.constprop.6+0x10/0x10
> > [ 7.275922] ? find_held_lock+0x32/0x1c0
> > [ 7.275929] ? rtnetlink_put_metrics+0x460/0x460
> > [ 7.275935] ? virtqueue_add_sgs+0x9e2/0xde0
> > [ 7.275953] ? virtscsi_add_cmd+0x454/0x780
> > [ 7.275964] ? find_held_lock+0x32/0x1c0
> > [ 7.275973] ? deref_stack_reg+0xad/0xe0
> > [ 7.275979] ? __read_once_size_nocheck.constprop.6+0x10/0x10
> > [ 7.275985] ? lock_downgrade+0x5e0/0x5e0
> > [ 7.275993] ? memset+0x1f/0x40
> > [ 7.276008] ? nla_parse+0x33/0x290
> > [ 7.276016] rtnl_newlink+0x954/0x1120
> > [ 7.276030] ? rtnl_link_unregister+0x250/0x250
> > [ 7.276044] ? is_bpf_text_address+0x5/0x60
> > [ 7.276054] ? lock_downgrade+0x5e0/0x5e0
> > [ 7.276057] ? lock_acquire+0x10b/0x2a0
> > [ 7.276072] ? deref_stack_reg+0xad/0xe0
> > [ 7.276078] ? __read_once_size_nocheck.constprop.6+0x10/0x10
> > [ 7.276085] ? __kernel_text_address+0xe/0x30
> > [ 7.276090] ? unwind_get_return_address+0x5f/0xa0
> > [ 7.276103] ? find_held_lock+0x32/0x1c0
> > [ 7.276110] ? is_bpf_text_address+0x5/0x60
> > [ 7.276124] ? deref_stack_reg+0xad/0xe0
> > [ 7.276131] ? __read_once_size_nocheck.constprop.6+0x10/0x10
> > [ 7.276136] ? depot_save_stack+0x2d9/0x460
> > [ 7.276142] ? deref_stack_reg+0xad/0xe0
> > [ 7.276156] ? find_held_lock+0x32/0x1c0
> > [ 7.276164] ? is_bpf_text_address+0x5/0x60
> > [ 7.276170] ? __lock_acquire.isra.29+0xe8/0x1bb0
> > [ 7.276212] ? rtnetlink_rcv_msg+0x5d6/0x930
> > [ 7.276222] ? lock_downgrade+0x5e0/0x5e0
> > [ 7.276226] ? lock_acquire+0x10b/0x2a0
> > [ 7.276240] rtnetlink_rcv_msg+0x69e/0x930
> > [ 7.276249] ? rtnl_calcit.isra.31+0x2f0/0x2f0
> > [ 7.276255] ? __lock_acquire.isra.29+0xe8/0x1bb0
> > [ 7.276265] ? netlink_deliver_tap+0x8d/0x8e0
> > [ 7.276276] netlink_rcv_skb+0x127/0x350
> > [ 7.276281] ? rtnl_calcit.isra.31+0x2f0/0x2f0
> > [ 7.276289] ? netlink_ack+0x970/0x970
> > [ 7.276299] ? __alloc_skb+0xc2/0x520
> > [ 7.276311] netlink_unicast+0x40f/0x5d0
> > [ 7.276320] ? netlink_attachskb+0x580/0x580
> > [ 7.276325] ? _copy_from_iter_full+0x157/0x6f0
> > [ 7.276331] ? import_iovec+0x90/0x390
> > [ 7.276338] ? get_page_from_freelist+0x1e89/0x3e30
> > [ 7.276347] ? apparmor_socket_sock_rcv_skb+0x10/0x10
> > [ 7.276357] netlink_sendmsg+0x65e/0xb00
> > [ 7.276367] ? netlink_unicast+0x5d0/0x5d0
> > [ 7.276373] ? copy_msghdr_from_user+0x206/0x340
> > [ 7.276388] ? netlink_unicast+0x5d0/0x5d0
> > [ 7.276394] sock_sendmsg+0xb3/0xf0
> > [ 7.276401] ___sys_sendmsg+0x604/0x8b0
> > [ 7.276410] ? copy_msghdr_from_user+0x340/0x340
> > [ 7.276416] ? find_held_lock+0x32/0x1c0
> > [ 7.276424] ? __handle_mm_fault+0xc85/0x3140
> > [ 7.276433] ? lock_downgrade+0x5e0/0x5e0
> > [ 7.276439] ? mem_cgroup_commit_charge+0xb4/0xf80
> > [ 7.276453] ? _raw_spin_unlock+0x24/0x30
> > [ 7.276458] ? __handle_mm_fault+0xc85/0x3140
> > [ 7.276467] ? __pmd_alloc+0x430/0x430
> > [ 7.276473] ? find_held_lock+0x32/0x1c0
> > [ 7.276485] ? __fget_light+0x55/0x1f0
> > [ 7.276497] ? __sys_sendmsg+0xd2/0x170
> > [ 7.276502] __sys_sendmsg+0xd2/0x170
> > [ 7.276508] ? __ia32_sys_shutdown+0x70/0x70
> > [ 7.276516] ? handle_mm_fault+0x1f9/0x6a0
> > [ 7.276528] ? up_read+0x1c/0x110
> > [ 7.276534] ? __do_page_fault+0x4a6/0xa80
> > [ 7.276554] do_syscall_64+0xa0/0x280
> > [ 7.276558] ? prepare_exit_to_usermode+0x88/0x130
> > [ 7.276566] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > [ 7.276572] RIP: 0033:0x7ffbe9a2f160
> > [ 7.276574] Code: c3 48 8b 05 2a 2d 2c 00 f7 db 64 89 18 48 83 cb ff eb dd 0f 1f 80 00 00 00 00 83 3d 1d 8f 2c 00 00 75 10 b8 2e 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 ee cb 00 00 48 89 04 24
> > [ 7.276728] RSP: 002b:00007ffc5a2d6108 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> > [ 7.276735] RAX: ffffffffffffffda RBX: 00007ffc5a2da220 RCX: 00007ffbe9a2f160
> > [ 7.276738] RDX: 0000000000000000 RSI: 00007ffc5a2d6150 RDI: 0000000000000003
> > [ 7.276741] RBP: 00007ffc5a2d6150 R08: 0000000000000000 R09: 0000000000000003
> > [ 7.276744] R10: 00007ffc5a2d5ed0 R11: 0000000000000246 R12: 000000005b6177de
> > [ 7.276748] R13: 0000000000000000 R14: 00000000006473a0 R15: 00007ffc5a2da918
> > [ 7.276763]
> > [ 7.276895] Allocated by task 1:
> > [ 7.277026] kasan_kmalloc+0xa0/0xd0
> > [ 7.277030] __kmalloc+0x13a/0x250
> > [ 7.277034] init_vqs+0xd0/0x11c0
> > [ 7.277038] virtnet_probe+0xb99/0x1ad0
> > [ 7.277045] virtio_dev_probe+0x3fc/0x890
> > [ 7.277052] driver_probe_device+0x6c4/0xcc0
> > [ 7.277056] __driver_attach+0x232/0x2c0
> > [ 7.277060] bus_for_each_dev+0x118/0x1a0
> > [ 7.277064] bus_add_driver+0x390/0x6e0
> > [ 7.277068] driver_register+0x18e/0x400
> > [ 7.277076] virtio_net_driver_init+0x6d/0x90
> > [ 7.277080] do_one_initcall+0xa8/0x348
> > [ 7.277085] kernel_init_freeable+0x42d/0x4c8
> > [ 7.277090] kernel_init+0xf/0x130
> > [ 7.277095] ret_from_fork+0x35/0x40
> > [ 7.277097]
> > [ 7.277223] Freed by task 0:
> > [ 7.277347] (stack is not available)
> > [ 7.277473]
> > [ 7.277596] The buggy address belongs to the object at ffff8801d4449100
> > [ 7.277596] which belongs to the cache kmalloc-4096 of size 4096
> > [ 7.277769] The buggy address is located 3840 bytes inside of
> > [ 7.277769] 4096-byte region [ffff8801d4449100, ffff8801d444a100)
> > [ 7.277932] The buggy address belongs to the page:
> > [ 7.278066] page:ffffea0007511200 count:1 mapcount:0 mapping:ffff8801db002600 index:0x0 compound_mapcount: 0
> > [ 7.278230] flags: 0x17fff8000008100(slab|head)
> > [ 7.278363] raw: 017fff8000008100 dead000000000100 dead000000000200 ffff8801db002600
> > [ 7.278516] raw: 0000000000000000 0000000000070007 00000001ffffffff 0000000000000000
> > [ 7.278664] page dumped because: kasan: bad access detected
> > [ 7.278790]
> > [ 7.278904] Memory state around the buggy address:
> > [ 7.279031] ffff8801d4449f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > [ 7.279175] ffff8801d4449f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > [ 7.279323] >ffff8801d444a000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > [ 7.279468] ^
> > [ 7.279584] ffff8801d444a080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > [ 7.279729] ffff8801d444a100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > [ 7.279870] ==================================================================
> > [ 7.280011] Disabling lock debugging due to kernel taint
> > [ 7.632219] random: mktemp: uninitialized urandom read (6 bytes read)
> > [ 8.052850] random: mktemp: uninitialized urandom read (6 bytes read)
> > [ 8.335209] random: cloud-init: uninitialized urandom read (32 bytes read)
> > [ 8.796331] random: cloud-init: uninitialized urandom read (32 bytes read)
> > [ 9.162551] random: mktemp: uninitialized urandom read (12 bytes read)
> > [ 9.384504] random: ssh-keygen: uninitialized urandom read (32 bytes read)
> > [ 9.839586] init: failsafe main process (724) killed by TERM signal
> > [ 14.865443] postgres (1245): /proc/1245/oom_adj is deprecated, please use /proc/1245/oom_score_adj instead.
> > [ 17.213418] random: crng init done
> > [ 17.580892] init: plymouth-upstart-bridge main process ended, respawning
>
> I suspect an off by one somewhere. I'm looking at the patch
> and I don't see it but these things are hard to spot sometimes ...
This bug was fixed in this commit:
commit ca9e83b4a55bfa1cc1395b48c3bf70381833526b
Author: Jason Wang <jasowang@redhat.com>
Date: Tue Jul 31 17:43:38 2018 +0800
virtio-net: correctly update XDP_TX counters
>
> > >
> > > > >> But this may not a
> > > > >> clean solution. It'd help if I can get suggestions on what would be a
> > > > >> clean option to fix this without extensively changing the code in
> > > > >> virtio_net. Is it mandatory to protect the affinitization with
> > > > >> read_lock? I don't see similar lock in other drivers while setting the
> > > > >> affinity. I understand this warning should go away, but isn't it safe to
> > > > >> have multiple readers.
> > > > >>
> > > > >>> On Fri, Jun 29, 2018 at 09:27:07PM -0700, Amritha Nambiar wrote:
^ permalink raw reply
* Re: KCM - recvmsg() mangles packets?
From: Dominique Martinet @ 2018-08-04 2:08 UTC (permalink / raw)
To: Tom Herbert; +Cc: Linux Kernel Network Developers
In-Reply-To: <20180804014132.GA26606@nautica>
Dominique Martinet wrote on Sat, Aug 04, 2018:
> Actually, now I'm looking closer to the timing, it looks specific to the
> connection setup. This send loop works:
> int i = 1;
> while(i <= 1000) {
> int len = (i++ * 1312739ULL) % 31 + 1;
> my_msg.hdr.len = htonl(len);
> for (int j = 0; j < len; ) {
> j += snprintf(my_msg.data + j, len - j,
> "%i", i - 1);
> }
> my_msg.data[len-1] = '\0';
> //printf("%d: writing %d\n", i-1, len);
> len = write(s, &my_msg, sizeof(my_msg.hdr) + len);
> if (error == -1)
> err(EXIT_FAILURE, "write");
> if (i == 2)
> usleep(1);
> }
>
> But removing the usleep(1) after the first packet makes recvmsg()
> "fail": it reads the content of the second packet with the size of the
> first.
I talked too fast, I can get this to fail on later packets e.g.
Got 18, expected 31 on 452nd message: 453453453453453453; flags: 80
The content is 453 in a loop so this really is the 453rd packet...
But being slower e.g. doing that usleep after every single packets and I
could let the loop run until 100k without a hintch.
There really has to be something wrong, I just can't tell what from
looking at the code with my naive eyes.
Maybe we need to lock both the tcp and the kcm sockets?
Thanks,
--
Dominique
^ permalink raw reply
* Re: KCM - recvmsg() mangles packets?
From: Dominique Martinet @ 2018-08-04 1:41 UTC (permalink / raw)
To: Tom Herbert; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAPDqMeo_OW29=5oT8-YBvrD-sg1uNmPn3wSJtKWAgF2LRwLKWA@mail.gmail.com>
Tom Herbert wrote on Fri, Aug 03, 2018:
> On Fri, Aug 3, 2018 at 4:20 PM, Dominique Martinet
> <asmadeus@codewreck.org> wrote:
> > Tom Herbert wrote on Fri, Aug 03, 2018:
> >> struct my_proto {
> >> struct _hdr {
> >> uint32_t len;
> >> } hdr;
> >> char data[32];
> >> } __attribute__((packed));
> >>
> >> // use htons to use LE header size, since load_half does a first convertion
> >> // from network byte order
> >> const char *bpf_prog_string = " \
> >> ssize_t bpf_prog1(struct __sk_buff *skb) \
> >> { \
> >> return bpf_htons(load_half(skb, 0)) + 4; \
> >> }";
> >
> > (Just to make sure I did fix it to htonl(load_word()) and I can confirm
> > there is no difference)
>
> You also need to htonl for
>
> my_msg.hdr.len = (i++ * 1312739ULL) % 31 + 1;
Thanks, but this looks correct to me - I was writing the header in
little endian order here and doing the double-swap dance in the bpf prog
because the protocol I was considering making a KCM implementation for
uses that.
Just to make sure, I rewrote it using network byte order e.g. these
three points and this makes no difference:
---8<----------------------
diff --git a/kcm.c b/kcm.c
index cb48df1..d437226 100644
--- a/kcm.c
+++ b/kcm.c
@@ -36,7 +36,7 @@ struct my_proto {
const char *bpf_prog_string = " \
ssize_t bpf_prog1(struct __sk_buff *skb) \
{ \
- return bpf_htons(load_half(skb, 0)) + 4; \
+ return load_word(skb, 0) + 4; \
}";
int servsock_init(int port)
@@ -110,13 +110,15 @@ void client(int port)
int i = 1;
while(1) {
- my_msg.hdr.len = (i++ * 1312739ULL) % 31 + 1;
- for (int j = 0; j < my_msg.hdr.len; ) {
- j += snprintf(my_msg.data + j, my_msg.hdr.len - j, "%i", i - 1);
+ int len = (i++ * 1312739ULL) % 31 + 1;
+ my_msg.hdr.len = htonl(len);
+ for (int j = 0; j < len; ) {
+ j += snprintf(my_msg.data + j, len - j,
+ "%i", i - 1);
}
- my_msg.data[my_msg.hdr.len-1] = '\0';
- //printf("%d: writing %d\n", i-1, my_msg.hdr.len);
- len = write(s, &my_msg, sizeof(my_msg.hdr) + my_msg.hdr.len);
+ my_msg.data[len-1] = '\0';
+ //printf("%d: writing %d\n", i-1, len);
+ len = write(s, &my_msg, sizeof(my_msg.hdr) + len);
if (error == -1)
err(EXIT_FAILURE, "write");
//usleep(10000);
@@ -171,9 +173,10 @@ void process(int kcmfd)
len = recvmsg(kcmfd, &msg, 0);
if (len == -1)
err(EXIT_FAILURE, "recvmsg");
- if (len != my_msg.hdr.len + 4) {
+ if (len != ntohl(my_msg.hdr.len) + 4) {
printf("Got %d, expected %d on %dth message: %s; flags:
%x\n",
- len - 4, my_msg.hdr.len, i, my_msg.data, msg.msg_flags);
+ len - 4, ntohl(my_msg.hdr.len), i,
+ my_msg.data, msg.msg_flags);
exit(1);
}
i++;
----------------8<-----------
Frankly I do not believe this is a rule problem, as if the length
splitting was incorrect the program would not work at all, but just
uncommenting the usleep on the sender side makes this work.
Actually, now I'm looking closer to the timing, it looks specific to the
connection setup. This send loop works:
int i = 1;
while(i <= 1000) {
int len = (i++ * 1312739ULL) % 31 + 1;
my_msg.hdr.len = htonl(len);
for (int j = 0; j < len; ) {
j += snprintf(my_msg.data + j, len - j,
"%i", i - 1);
}
my_msg.data[len-1] = '\0';
//printf("%d: writing %d\n", i-1, len);
len = write(s, &my_msg, sizeof(my_msg.hdr) + len);
if (error == -1)
err(EXIT_FAILURE, "write");
if (i == 2)
usleep(1);
}
But removing the usleep(1) after the first packet makes recvmsg()
"fail": it reads the content of the second packet with the size of the
first.
I assume that usleep gives the server time to finish setting up the kcm
socket, because it does accept(); ioctl(SIOCKCMATTACH); recvmsg(); but
the client does not wait to send packets so there could be some sort of
race with the attach and multiple packets?
FWIW I took the time to look at older kernel and this has been happening
ever since KCM got introduced in 4.6
Thanks,
--
Dominique
^ permalink raw reply related
* [stmmac][bug?] endianness of Flexible RX Parser code
From: Al Viro @ 2018-08-04 1:19 UTC (permalink / raw)
To: Jose Abreu; +Cc: David S. Miller, netdev
The values passed in struct tc_u32_sel ->mask and ->val are
32bit net-endian. Your tc_fill_entry() does this:
data = sel->keys[0].val;
mask = sel->keys[0].mask;
...
entry->frag_ptr = frag;
entry->val.match_en = (mask << (rem * 8)) &
GENMASK(31, rem * 8);
entry->val.match_data = (data << (rem * 8)) &
GENMASK(31, rem * 8);
entry->val.frame_offset = real_off;
entry->prio = prio;
frag->val.match_en = (mask >> (rem * 8)) &
GENMASK(rem * 8 - 1, 0);
frag->val.match_data = (data >> (rem * 8)) &
GENMASK(rem * 8 - 1, 0);
frag->val.frame_offset = real_off + 1;
frag->prio = prio;
frag->is_frag = true;
and that looks very odd. rem here is offset modulo 4. Suppose offset is
equal to 5, val contains {V0, V1, V2, V3} and mask - {M0, M1, M2, M3}.
Then on little-endian host we get
entry->val.match_en: {0, M0, M1, M2}
entry->val.match_data: {0, V0, V1, V2}
entry->val.frame_offset = 1;
frag->val.match_en: {M3, 0, 0, 0}
frag->val.match_data: {V3, 0, 0, 0}
frag->val.frame_offset = 2;
and on big-endian
entry->val.match_en: {M1, M2, M3, 0}
entry->val.match_data: {V1, V2, V3, 0}
entry->val.frame_offset = 1;
frag->val.match_en: {0, 0, 0, M0}
frag->val.match_data: {0, 0, 0, V0}
frag->val.frame_offset = 2;
Little-endian variant looks like we mask octets 5, 6, 7 and 8 with
M0..M3 resp. and want V0..V3 in those. On big-endian, though, we
look at the octets 11, 4, 5 and 6 instead.
I don't know the hardware (and it might be pulling any kind of weird
endianness-dependent stunts), but that really smells like a bug.
It looks like that code is trying to do something like
data = ntohl(sel->keys[0].val);
mask = ntohl(sel->keys[0].mask);
shift = rem * 8;
entry->val.match_en = htonl(mask >> shift);
entry->val.match_data = htonl(data >> shift);
entry->val.frame_offset = real_off;
...
frag->val.match_en = htonl(mask << (32 - shift));
frag->val.match_data = htonl(data << (32 - shift));
entry->val.frame_offset = real_off + 1;
Comments?
^ permalink raw reply
* Re: KCM - recvmsg() mangles packets?
From: Tom Herbert @ 2018-08-04 1:18 UTC (permalink / raw)
To: Dominique Martinet; +Cc: Linux Kernel Network Developers
In-Reply-To: <20180803232057.GB7583@nautica>
On Fri, Aug 3, 2018 at 4:20 PM, Dominique Martinet
<asmadeus@codewreck.org> wrote:
> Tom Herbert wrote on Fri, Aug 03, 2018:
>> struct my_proto {
>> struct _hdr {
>> uint32_t len;
>> } hdr;
>> char data[32];
>> } __attribute__((packed));
>>
>> // use htons to use LE header size, since load_half does a first convertion
>> // from network byte order
>> const char *bpf_prog_string = " \
>> ssize_t bpf_prog1(struct __sk_buff *skb) \
>> { \
>> return bpf_htons(load_half(skb, 0)) + 4; \
>> }";
>>
>> The length in hdr is uint32_t above, but this looks like it's being
>> read as a short.
>
> Err, I agree this is obviously wrong here (I can blame my lack of
> attention to this and the example I used), but this isn't the problem as
> the actual size is between 0 and 32 -- I could use any size I want here
> and the result would the same.
>
> A "real" problem with the conversion program would mean that my example
> would not work if I slow it down, but I can send as many packet as I
> want if I uncomment the usleep() on the client side or if I just
> throttle the network stack with a loud tcpdump writing to stdout -- that
> means the algorithm is working even if it's making some badly-sized
> conversions.
>
> (Just to make sure I did fix it to htonl(load_word()) and I can confirm
> there is no difference)
>
You also need to htonl for
my_msg.hdr.len = (i++ * 1312739ULL) % 31 + 1;
>
> Thanks,
> --
> Dominique Martinet
^ permalink raw reply
* Re: [PATCH] net: cisco: enic: Replace GFP_ATOMIC with GFP_KERNEL
From: Govindarajulu Varadarajan (gvaradar) @ 2018-08-04 0:54 UTC (permalink / raw)
To: baijiaju1990@gmail.com, _govind@gmx.com,
Christian Benvenuti (benve), Parvi Kaustubhi (pkaustub)
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20180804004009.18928-1-baijiaju1990@gmail.com>
On Sat, 2018-08-04 at 08:40 +0800, Jia-Ju Bai wrote:
> vnic_dev_register(), vnic_rq_alloc_bufs() and vnic_wq_alloc_bufs()
> are never called in atomic context.
> They call kzalloc() with GFP_ATOMIC, which is not necessary.
> GFP_ATOMIC can be replaced with GFP_KERNEL.
>
> This is found by a static analysis tool named DCNS written by myself.
Looks good to me. Thanks you.
Acked-by: Govindarajulu Varadarajan <gvaradar@cisco.com>
^ permalink raw reply
* Re: [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order
From: Stefano Brivio @ 2018-08-04 0:43 UTC (permalink / raw)
To: Ben Pfaff
Cc: Matteo Croce, Pravin B Shelar, jpettit, gvrose8192, netdev, dev,
Jiri Benc, Aaron Conole
In-Reply-To: <20180803230108.GU29662@ovn.org>
On Fri, 3 Aug 2018 16:01:08 -0700
Ben Pfaff <blp@ovn.org> wrote:
> I think that a simple mechanism for fairness is fine. The direction
> of extensibility that makes me anxious is how to decide what matters
> for fairness. So far, we've talked about per-vport fairness. That
> works pretty well for packets coming in from virtual interfaces where
> each vport represents a separate VM.
Yes, right, that's the case where we have significant issues currently.
> It does not work well if the traffic filling your queues all comes
> from a single physical port because some source of traffic is sending
> traffic at a high rate. In that case, you'll do a lot better if you
> do fairness based on the source 5-tuple. But if you're doing network
> virtualization, then the outer source 5-tuples won't necessarily vary
> much and you'd be better off looking at the VNI and maybe some Geneve
> TLV options and maybe the inner 5-tuple...
Sure, I see what you mean now. That looks entirely doable if we
abstract the round-robin bucket selection out of the current patch.
> I would be very pleased if we could integrate a simple mechanism for
> fairness, based for now on some simple criteria like the source port,
> but thinking ahead to how we could later make it gracefully extensible
> to consider more general and possibly customizable criteria.
We could change the patch so that instead of just using the vport for
round-robin queue insertion, we generalise that and use "buckets"
instead of vports, and have a set of possible functions that are called
instead of using port_no directly in ovs_dp_upcall_queue_roundrobin(),
making this configurable via netlink, per datapath.
We could implement selection based on source port or a hash on the
source 5-tuple, and the relevant bits of
ovs_dp_upcall_queue_roundrobin() would look like this:
static int ovs_dp_upcall_queue_roundrobin(struct datapath *dp,
struct dp_upcall_info *upcall)
{
[...]
list_for_each_entry(pos, head, list) {
int bucket = dp->rr_select(pos);
/* Count per-bucket upcalls. */
if (dp->upcalls.count[bucket] == U8_MAX) {
err = -ENOSPC;
goto out_clear;
}
dp->upcalls.count[bucket]++;
if (bucket == upcall->bucket) {
/* Another upcall for the same bucket: move insertion
* point here, keep looking for insertion condition to
* be still met further on.
*/
find_next = true;
here = pos;
continue;
}
count = dp->upcalls.count[bucket];
if (find_next && dp->upcalls.count[bucket] >= count) {
/* Insertion condition met: no need to look further,
* unless another upcall for the same port occurs later.
*/
find_next = false;
here = pos;
}
}
[...]
}
and implementations for dp->rr_select() would look like:
int rr_select_vport(struct dp_upcall_info *upcall)
{
return upcall->port_no;
}
int rr_select_srcport(struct dp_upcall_info *upcall)
{
/* look up source port from upcall->skb... */
}
And we could then easily extend this to use BPF with maps one day.
This is for clarity by the way, but I guess we should avoid indirect
calls in the final implementation.
What do you think?
--
Stefano
^ permalink raw reply
* Hello
From: alihamadu04@gmail.com @ 2018-08-04 0:18 UTC (permalink / raw)
Dear Friend,
Please i need your kind Assistance. I will be very glad if you can
assist me to receive this sum of ( $22. Million US dollars.) into your
bank account for the benefit of our both families, reply me if you are
ready to receive this fund.
^ permalink raw reply
* Hello
From: alihamadu04@gmail.com @ 2018-08-04 0:18 UTC (permalink / raw)
Dear Friend,
Please i need your kind Assistance. I will be very glad if you can
assist me to receive this sum of ( $22. Million US dollars.) into your
bank account for the benefit of our both families, reply me if you are
ready to receive this fund.
^ permalink raw reply
* Re: [PATCH net 0/5] tcp: more robust ooo handling
From: David Miller @ 2018-08-03 23:53 UTC (permalink / raw)
To: dwmw2
Cc: edumazet, juha-matti.tilli, ycheng, soheil, netdev, eric.dumazet,
gregkh
In-Reply-To: <1533293737.16128.105.camel@infradead.org>
From: David Woodhouse <dwmw2@infradead.org>
Date: Fri, 03 Aug 2018 11:55:37 +0100
> I see the first four in 4.9.116 but not the fifth (adding
> tcp_ooo_try_coalesce()).
>
> Is that intentional?
I don't work on the 4.9 -stable backports, so I personally have
no idea.
I submitted for 4.17 and 4.14
^ permalink raw reply
* Re: [PATCH v2 net-next] af_unix: ensure POLLOUT on remote close() for connected dgram socket
From: David Miller @ 2018-08-03 23:44 UTC (permalink / raw)
To: jbaron; +Cc: netdev, rientjes, rweikusat, edumazet
In-Reply-To: <1533331493-26286-1-git-send-email-jbaron@akamai.com>
From: Jason Baron <jbaron@akamai.com>
Date: Fri, 3 Aug 2018 17:24:53 -0400
> Applications use -ECONNREFUSED as returned from write() in order to
> determine that a socket should be closed. However, when using connected
> dgram unix sockets in a poll/write loop, a final POLLOUT event can be
> missed when the remote end closes. Thus, the poll is stuck forever:
>
> thread 1 (client) thread 2 (server)
>
> connect() to server
> write() returns -EAGAIN
> unix_dgram_poll()
> -> unix_recvq_full() is true
> close()
> ->unix_release_sock()
> ->wake_up_interruptible_all()
> unix_dgram_poll() (due to the
> wake_up_interruptible_all)
> -> unix_recvq_full() still is true
> ->free all skbs
>
>
> Now thread 1 is stuck and will not receive anymore wakeups. In this
> case, when thread 1 gets the -EAGAIN, it has not queued any skbs
> otherwise the 'free all skbs' step would in fact cause a wakeup and
> a POLLOUT return. So the race here is probably fairly rare because
> it means there are no skbs that thread 1 queued and that thread 1
> schedules before the 'free all skbs' step.
>
> This issue was reported as a hang when /dev/log is closed.
>
> The fix is to signal POLLOUT if the socket is marked as SOCK_DEAD, which
> means a subsequent write() will get -ECONNREFUSED.
>
> Reported-by: Ian Lance Taylor <iant@golang.org>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Rainer Weikusat <rweikusat@mobileactivedefense.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Jason Baron <jbaron@akamai.com>
> ---
> v2: use check for SOCK_DEAD, since skb's can be purged in unix_sock_destructor()
Applied, thanks Jason.
^ permalink raw reply
* Re: [PATCH v2 0/2] net/sctp: Avoid allocating high order memory with kmalloc()
From: Marcelo Ricardo Leitner @ 2018-08-03 23:36 UTC (permalink / raw)
To: Konstantin Khorenko
Cc: oleg.babin, netdev, linux-sctp, David S . Miller, Vlad Yasevich,
Neil Horman, Xin Long, Andrey Ryabinin
In-Reply-To: <20180803162102.19540-1-khorenko@virtuozzo.com>
On Fri, Aug 03, 2018 at 07:21:00PM +0300, Konstantin Khorenko wrote:
...
> Performance results:
> ====================
> * Kernel: v4.18-rc6 - stock and with 2 patches from Oleg (earlier in this thread)
> * Node: CPU (8 cores): Intel(R) Xeon(R) CPU E31230 @ 3.20GHz
> RAM: 32 Gb
>
> * netperf: taken from https://github.com/HewlettPackard/netperf.git,
> compiled from sources with sctp support
> * netperf server and client are run on the same node
> * ip link set lo mtu 1500
>
> The script used to run tests:
> # cat run_tests.sh
> #!/bin/bash
>
> for test in SCTP_STREAM SCTP_STREAM_MANY SCTP_RR SCTP_RR_MANY; do
> echo "TEST: $test";
> for i in `seq 1 3`; do
> echo "Iteration: $i";
> set -x
> netperf -t $test -H localhost -p 22222 -S 200000,200000 -s 200000,200000 \
> -l 60 -- -m 1452;
> set +x
> done
> done
> ================================================
>
> Results (a bit reformatted to be more readable):
...
Nice, good numbers.
I'm missing some test that actually uses more than 1 stream. All tests
in netperf uses only 1 stream. They can use 1 or Many associations on
a socket, but not multiple streams. That means the numbers here show
that we shouldn't see any regression on the more traditional uses, per
Michael's reply on the other email, but it is not testing how it will
behave if we go crazy and use the 64k streams (worst case).
You'll need some other tool to test it. One idea is sctp_test, from
lksctp-tools. Something like:
Server side:
./sctp_test -H 172.0.0.1 -P 22222 -l -d 0
Client side:
time ./sctp_test -H 172.0.0.1 -P 22221 \
-h 172.0.0.1 -p 22222 -s \
-c 1 -M 65535 -T -t 1 -x 100000 -d 0
And then measure the difference on how long each test took. Can you
get these too?
Interesting that in my laptop just to start this test for the first
time can took some *seconds*. Seems kernel had a hard time
defragmenting the memory here. :)
Thanks,
Marcelo
^ permalink raw reply
* Re: KCM - recvmsg() mangles packets?
From: Dominique Martinet @ 2018-08-03 23:20 UTC (permalink / raw)
To: Tom Herbert; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAPDqMeorHCcYKmbobH4biiZdoWf34Mw4BUk_L1deSTRK2TSmRA@mail.gmail.com>
Tom Herbert wrote on Fri, Aug 03, 2018:
> struct my_proto {
> struct _hdr {
> uint32_t len;
> } hdr;
> char data[32];
> } __attribute__((packed));
>
> // use htons to use LE header size, since load_half does a first convertion
> // from network byte order
> const char *bpf_prog_string = " \
> ssize_t bpf_prog1(struct __sk_buff *skb) \
> { \
> return bpf_htons(load_half(skb, 0)) + 4; \
> }";
>
> The length in hdr is uint32_t above, but this looks like it's being
> read as a short.
Err, I agree this is obviously wrong here (I can blame my lack of
attention to this and the example I used), but this isn't the problem as
the actual size is between 0 and 32 -- I could use any size I want here
and the result would the same.
A "real" problem with the conversion program would mean that my example
would not work if I slow it down, but I can send as many packet as I
want if I uncomment the usleep() on the client side or if I just
throttle the network stack with a loud tcpdump writing to stdout -- that
means the algorithm is working even if it's making some badly-sized
conversions.
(Just to make sure I did fix it to htonl(load_word()) and I can confirm
there is no difference)
Thanks,
--
Dominique Martinet
^ permalink raw reply
* [linux-next:master 11347/11797] include/linux/compiler.h:61:17: warning: 'ib_query_gid' is deprecated
From: kbuild test robot @ 2018-08-03 23:12 UTC (permalink / raw)
To: David S. Miller; +Cc: kbuild-all, netdev
[-- Attachment #1: Type: text/plain, Size: 7016 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head: 116b181bb646afedd770985de20a68721bdb2648
commit: 9f4c2b1ceca8021c68c23e655b275c693367a48c [11347/11797] next-20180802/net-next
config: x86_64-randconfig-s1-08040602 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
git checkout 9f4c2b1ceca8021c68c23e655b275c693367a48c
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
net/smc/smc_ib.c: In function 'smc_ib_fill_mac':
net/smc/smc_ib.c:152:2: warning: 'ib_query_gid' is deprecated [-Wdeprecated-declarations]
rc = ib_query_gid(smcibdev->ibdev, ibport, 0, &gid, &gattr);
^~
In file included from net/smc/smc_ib.c:19:0:
include/rdma/ib_cache.h:139:32: note: declared here
static inline __deprecated int ib_query_gid(struct ib_device *device,
^~~~~~~~~~~~
net/smc/smc_ib.c: In function 'smc_ib_determine_gid':
net/smc/smc_ib.c:190:3: warning: 'ib_query_gid' is deprecated [-Wdeprecated-declarations]
if (ib_query_gid(smcibdev->ibdev, ibport, i, &_gid, &gattr))
^~
In file included from net/smc/smc_ib.c:19:0:
include/rdma/ib_cache.h:139:32: note: declared here
static inline __deprecated int ib_query_gid(struct ib_device *device,
^~~~~~~~~~~~
net/smc/smc_ib.c:190:3: warning: 'ib_query_gid' is deprecated [-Wdeprecated-declarations]
if (ib_query_gid(smcibdev->ibdev, ibport, i, &_gid, &gattr))
^~
In file included from net/smc/smc_ib.c:19:0:
include/rdma/ib_cache.h:139:32: note: declared here
static inline __deprecated int ib_query_gid(struct ib_device *device,
^~~~~~~~~~~~
In file included from include/linux/kernel.h:10:0,
from include/linux/list.h:9,
from include/linux/random.h:10,
from net/smc/smc_ib.c:15:
>> include/linux/compiler.h:61:17: warning: 'ib_query_gid' is deprecated [-Wdeprecated-declarations]
static struct ftrace_branch_data \
^
include/linux/compiler.h:56:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
>> net/smc/smc_ib.c:190:3: note: in expansion of macro 'if'
if (ib_query_gid(smcibdev->ibdev, ibport, i, &_gid, &gattr))
^~
In file included from net/smc/smc_ib.c:19:0:
include/rdma/ib_cache.h:139:32: note: declared here
static inline __deprecated int ib_query_gid(struct ib_device *device,
^~~~~~~~~~~~
--
net//smc/smc_ib.c: In function 'smc_ib_fill_mac':
net//smc/smc_ib.c:152:2: warning: 'ib_query_gid' is deprecated [-Wdeprecated-declarations]
rc = ib_query_gid(smcibdev->ibdev, ibport, 0, &gid, &gattr);
^~
In file included from net//smc/smc_ib.c:19:0:
include/rdma/ib_cache.h:139:32: note: declared here
static inline __deprecated int ib_query_gid(struct ib_device *device,
^~~~~~~~~~~~
net//smc/smc_ib.c: In function 'smc_ib_determine_gid':
net//smc/smc_ib.c:190:3: warning: 'ib_query_gid' is deprecated [-Wdeprecated-declarations]
if (ib_query_gid(smcibdev->ibdev, ibport, i, &_gid, &gattr))
^~
In file included from net//smc/smc_ib.c:19:0:
include/rdma/ib_cache.h:139:32: note: declared here
static inline __deprecated int ib_query_gid(struct ib_device *device,
^~~~~~~~~~~~
net//smc/smc_ib.c:190:3: warning: 'ib_query_gid' is deprecated [-Wdeprecated-declarations]
if (ib_query_gid(smcibdev->ibdev, ibport, i, &_gid, &gattr))
^~
In file included from net//smc/smc_ib.c:19:0:
include/rdma/ib_cache.h:139:32: note: declared here
static inline __deprecated int ib_query_gid(struct ib_device *device,
^~~~~~~~~~~~
In file included from include/linux/kernel.h:10:0,
from include/linux/list.h:9,
from include/linux/random.h:10,
from net//smc/smc_ib.c:15:
>> include/linux/compiler.h:61:17: warning: 'ib_query_gid' is deprecated [-Wdeprecated-declarations]
static struct ftrace_branch_data \
^
include/linux/compiler.h:56:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
net//smc/smc_ib.c:190:3: note: in expansion of macro 'if'
if (ib_query_gid(smcibdev->ibdev, ibport, i, &_gid, &gattr))
^~
In file included from net//smc/smc_ib.c:19:0:
include/rdma/ib_cache.h:139:32: note: declared here
static inline __deprecated int ib_query_gid(struct ib_device *device,
^~~~~~~~~~~~
vim +/ib_query_gid +61 include/linux/compiler.h
2bcd521a Steven Rostedt 2008-11-21 50
2bcd521a Steven Rostedt 2008-11-21 51 #ifdef CONFIG_PROFILE_ALL_BRANCHES
2bcd521a Steven Rostedt 2008-11-21 52 /*
2bcd521a Steven Rostedt 2008-11-21 53 * "Define 'is'", Bill Clinton
2bcd521a Steven Rostedt 2008-11-21 54 * "Define 'if'", Steven Rostedt
2bcd521a Steven Rostedt 2008-11-21 55 */
ab3c9c68 Linus Torvalds 2009-04-07 56 #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
ab3c9c68 Linus Torvalds 2009-04-07 57 #define __trace_if(cond) \
b33c8ff4 Arnd Bergmann 2016-02-12 58 if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
2bcd521a Steven Rostedt 2008-11-21 59 ({ \
2bcd521a Steven Rostedt 2008-11-21 60 int ______r; \
2bcd521a Steven Rostedt 2008-11-21 @61 static struct ftrace_branch_data \
2bcd521a Steven Rostedt 2008-11-21 62 __attribute__((__aligned__(4))) \
2bcd521a Steven Rostedt 2008-11-21 63 __attribute__((section("_ftrace_branch"))) \
2bcd521a Steven Rostedt 2008-11-21 64 ______f = { \
2bcd521a Steven Rostedt 2008-11-21 65 .func = __func__, \
2bcd521a Steven Rostedt 2008-11-21 66 .file = __FILE__, \
2bcd521a Steven Rostedt 2008-11-21 67 .line = __LINE__, \
2bcd521a Steven Rostedt 2008-11-21 68 }; \
2bcd521a Steven Rostedt 2008-11-21 69 ______r = !!(cond); \
97e7e4f3 Witold Baryluk 2009-03-17 70 ______f.miss_hit[______r]++; \
2bcd521a Steven Rostedt 2008-11-21 71 ______r; \
2bcd521a Steven Rostedt 2008-11-21 72 }))
2bcd521a Steven Rostedt 2008-11-21 73 #endif /* CONFIG_PROFILE_ALL_BRANCHES */
2bcd521a Steven Rostedt 2008-11-21 74
:::::: The code at line 61 was first introduced by commit
:::::: 2bcd521a684cc94befbe2ce7d5b613c841b0d304 trace: profile all if conditionals
:::::: TO: Steven Rostedt <srostedt@redhat.com>
:::::: CC: Ingo Molnar <mingo@elte.hu>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27074 bytes --]
^ permalink raw reply
* Re: [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order
From: Ben Pfaff @ 2018-08-04 0:54 UTC (permalink / raw)
To: Stefano Brivio
Cc: dev-yBygre7rU0TnMu66kgdUjQ, jpettit-pghWNbHTmq7QT0dZR+AlfA,
netdev, Jiri Benc
In-Reply-To: <20180804024324.4d900b5e@epycfail>
On Sat, Aug 04, 2018 at 02:43:24AM +0200, Stefano Brivio wrote:
> On Fri, 3 Aug 2018 16:01:08 -0700
> Ben Pfaff <blp-LZ6Gd1LRuIk@public.gmane.org> wrote:
> > I would be very pleased if we could integrate a simple mechanism for
> > fairness, based for now on some simple criteria like the source port,
> > but thinking ahead to how we could later make it gracefully extensible
> > to consider more general and possibly customizable criteria.
>
> We could change the patch so that instead of just using the vport for
> round-robin queue insertion, we generalise that and use "buckets"
> instead of vports, and have a set of possible functions that are called
> instead of using port_no directly in ovs_dp_upcall_queue_roundrobin(),
> making this configurable via netlink, per datapath.
>
> We could implement selection based on source port or a hash on the
> source 5-tuple, and the relevant bits of
> ovs_dp_upcall_queue_roundrobin() would look like this:
[...]
> What do you think?
I'd support that. Thanks.
^ permalink raw reply
* Re: KCM - recvmsg() mangles packets?
From: Tom Herbert @ 2018-08-03 22:46 UTC (permalink / raw)
To: Dominique Martinet; +Cc: Linux Kernel Network Developers
In-Reply-To: <20180803182830.GB29193@nautica>
struct my_proto {
struct _hdr {
uint32_t len;
} hdr;
char data[32];
} __attribute__((packed));
// use htons to use LE header size, since load_half does a first convertion
// from network byte order
const char *bpf_prog_string = " \
ssize_t bpf_prog1(struct __sk_buff *skb) \
{ \
return bpf_htons(load_half(skb, 0)) + 4; \
}";
On Fri, Aug 3, 2018 at 11:28 AM, Dominique Martinet
<asmadeus@codewreck.org> wrote:
> I've been playing with KCM on a 4.18.0-rc7 kernel and I'm running in a
> problem where the iovec filled by recvmsg() is mangled up: it is filled
> by the length of one packet, but contains (truncated) data from another
> packet, rendering KCM unuseable.
>
> (I haven't tried old kernels to see for how long this is broken/try to
> bisect; I might if there's no progress but this might be simpler than I
> think)
>
>
> I've attached a reproducer, a simple program that forks, creates a tcp
> server/client, attach the server socket to a kcm socket, and in an
> infinite loop sends varying-length messages from the client to the
> server.
> The loop stops when the server gets a message which length is not the
> length indicated in the packet header, rather fast (I can make it run
> for a while if I slow down emission, or if I run a verbose tcpdump for
> example)
>
>From the reproducer:
struct my_proto {
struct _hdr {
uint32_t len;
} hdr;
char data[32];
} __attribute__((packed));
// use htons to use LE header size, since load_half does a first convertion
// from network byte order
const char *bpf_prog_string = " \
ssize_t bpf_prog1(struct __sk_buff *skb) \
{ \
return bpf_htons(load_half(skb, 0)) + 4; \
}";
The length in hdr is uint32_t above, but this looks like it's being
read as a short.
Tom
> In the quiet version on a VM on my laptop, I get this output:
> [root@f2 ~]# gcc -g -l bcc -o kcm kcm.c
> [root@f2 ~]# ./kcm
> client is starting
> server is starting
> server is receiving data
> Got 14, expected 27 on 1th message: 22222222222222; flags: 80
>
> The client sends message deterministacally, first one is 14 bytes filled
> with 1, second one is 27 bytes filled with 2, third one is 9 bytes
> filled with 3 etc (final digit is actually a \0 instead)
>
> As we can see, the server received 14 '2', and the header size matches
> the second message header, so something went wrong™.
> Flags 0x80 is MSG_EOR meaning recvmsg copied the full message.
>
>
>
> This happens even if I reduce the VMs CPU to 1, so I was thinking some
> irq messes with the sock between skb_peek and the actual copy of the
> data (as this deos work if I send slowly!), but even disabling
> irq/preempt doesn't seem to help so I'm not sure what to try next.
>
> Any idea?
>
>
> Thanks,
> --
> Dominique Martinet
^ permalink raw reply
* [PATCH] net: cisco: enic: Replace GFP_ATOMIC with GFP_KERNEL
From: Jia-Ju Bai @ 2018-08-04 0:40 UTC (permalink / raw)
To: benve, _govind, pkaustub; +Cc: netdev, linux-kernel, Jia-Ju Bai
vnic_dev_register(), vnic_rq_alloc_bufs() and vnic_wq_alloc_bufs()
are never called in atomic context.
They call kzalloc() with GFP_ATOMIC, which is not necessary.
GFP_ATOMIC can be replaced with GFP_KERNEL.
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
drivers/net/ethernet/cisco/enic/vnic_dev.c | 2 +-
drivers/net/ethernet/cisco/enic/vnic_rq.c | 2 +-
drivers/net/ethernet/cisco/enic/vnic_wq.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/vnic_dev.c b/drivers/net/ethernet/cisco/enic/vnic_dev.c
index 76cdd4c9d11f..5c08c4519695 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_dev.c
+++ b/drivers/net/ethernet/cisco/enic/vnic_dev.c
@@ -1071,7 +1071,7 @@ struct vnic_dev *vnic_dev_register(struct vnic_dev *vdev,
unsigned int num_bars)
{
if (!vdev) {
- vdev = kzalloc(sizeof(struct vnic_dev), GFP_ATOMIC);
+ vdev = kzalloc(sizeof(struct vnic_dev), GFP_KERNEL);
if (!vdev)
return NULL;
}
diff --git a/drivers/net/ethernet/cisco/enic/vnic_rq.c b/drivers/net/ethernet/cisco/enic/vnic_rq.c
index f8aa326d1d58..a3e7b003ada1 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_rq.c
+++ b/drivers/net/ethernet/cisco/enic/vnic_rq.c
@@ -35,7 +35,7 @@ static int vnic_rq_alloc_bufs(struct vnic_rq *rq)
unsigned int blks = VNIC_RQ_BUF_BLKS_NEEDED(count);
for (i = 0; i < blks; i++) {
- rq->bufs[i] = kzalloc(VNIC_RQ_BUF_BLK_SZ(count), GFP_ATOMIC);
+ rq->bufs[i] = kzalloc(VNIC_RQ_BUF_BLK_SZ(count), GFP_KERNEL);
if (!rq->bufs[i])
return -ENOMEM;
}
diff --git a/drivers/net/ethernet/cisco/enic/vnic_wq.c b/drivers/net/ethernet/cisco/enic/vnic_wq.c
index 090cc65658a3..eb75891974df 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_wq.c
+++ b/drivers/net/ethernet/cisco/enic/vnic_wq.c
@@ -35,7 +35,7 @@ static int vnic_wq_alloc_bufs(struct vnic_wq *wq)
unsigned int blks = VNIC_WQ_BUF_BLKS_NEEDED(count);
for (i = 0; i < blks; i++) {
- wq->bufs[i] = kzalloc(VNIC_WQ_BUF_BLK_SZ(count), GFP_ATOMIC);
+ wq->bufs[i] = kzalloc(VNIC_WQ_BUF_BLK_SZ(count), GFP_KERNEL);
if (!wq->bufs[i])
return -ENOMEM;
}
--
2.17.0
^ permalink raw reply related
* Re: [PATCH v1 2/3] zinc: Introduce minimal cryptography library
From: Jason A. Donenfeld @ 2018-08-03 22:10 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Eric Biggers, Linux Crypto Mailing List, LKML, Netdev,
David Miller, Andrew Lutomirski, Greg Kroah-Hartman, Samuel Neves,
Daniel J . Bernstein, Tanja Lange, Jean-Philippe Aumasson,
Karthikeyan Bhargavan
In-Reply-To: <CALCETrVziUpAU4nTaZ=t5ct=1jsWswWOy7KDNxXGPD1L=tMTGQ@mail.gmail.com>
On Fri, Aug 3, 2018 at 11:29 PM Andy Lutomirski <luto@amacapital.net> wrote:
> Gotcha. That was very hidden in the 24k lines. Please make this (and
> any similar goodies) be their own patches.
I know, sorry, and I certainly will have this split out. The above
code snippet was from the much much shorter WireGuard patch (3/3), but
indeed the simd_get/put/relax code is in the monster patch here, so
that'll certainly be split for v2.
>
> Also, please consider spelling it differently:
>
> simd_context_t simd_context = simd_get();
>
> Because we'll feel very silly the first time some architecture has
> more than one possible state. (It wouldn't be entirely insane for x86
> to distinguish between "no SIMD", "XMM only", and "go to town!", for
> example.)
That's a great idea. It'll also make it clearer that users shouldn't
just dump a raw "true" in there; raw bools are more tempting to abuse
like that.
^ permalink raw reply
* Re: [PATCH net-next] tcp: remove unneeded variable 'err'
From: David Miller @ 2018-08-03 23:52 UTC (permalink / raw)
To: yuehaibing; +Cc: edumazet, kuznet, yoshfuji, linux-kernel, netdev
In-Reply-To: <20180803082848.24120-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Fri, 3 Aug 2018 16:28:48 +0800
> variable 'err' is unmodified after initalization,
> so simply cleans up it and returns 0.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied, thanks.
^ permalink raw reply
* [PATCH v2 net-next] af_unix: ensure POLLOUT on remote close() for connected dgram socket
From: Jason Baron @ 2018-08-03 21:24 UTC (permalink / raw)
To: davem; +Cc: netdev, David Rientjes, Rainer Weikusat, Eric Dumazet
Applications use -ECONNREFUSED as returned from write() in order to
determine that a socket should be closed. However, when using connected
dgram unix sockets in a poll/write loop, a final POLLOUT event can be
missed when the remote end closes. Thus, the poll is stuck forever:
thread 1 (client) thread 2 (server)
connect() to server
write() returns -EAGAIN
unix_dgram_poll()
-> unix_recvq_full() is true
close()
->unix_release_sock()
->wake_up_interruptible_all()
unix_dgram_poll() (due to the
wake_up_interruptible_all)
-> unix_recvq_full() still is true
->free all skbs
Now thread 1 is stuck and will not receive anymore wakeups. In this
case, when thread 1 gets the -EAGAIN, it has not queued any skbs
otherwise the 'free all skbs' step would in fact cause a wakeup and
a POLLOUT return. So the race here is probably fairly rare because
it means there are no skbs that thread 1 queued and that thread 1
schedules before the 'free all skbs' step.
This issue was reported as a hang when /dev/log is closed.
The fix is to signal POLLOUT if the socket is marked as SOCK_DEAD, which
means a subsequent write() will get -ECONNREFUSED.
Reported-by: Ian Lance Taylor <iant@golang.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Rainer Weikusat <rweikusat@mobileactivedefense.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jason Baron <jbaron@akamai.com>
---
v2: use check for SOCK_DEAD, since skb's can be purged in unix_sock_destructor()
---
net/unix/af_unix.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 1772a0e..d1edfa3 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -430,7 +430,12 @@ static int unix_dgram_peer_wake_me(struct sock *sk, struct sock *other)
connected = unix_dgram_peer_wake_connect(sk, other);
- if (unix_recvq_full(other))
+ /* If other is SOCK_DEAD, we want to make sure we signal
+ * POLLOUT, such that a subsequent write() can get a
+ * -ECONNREFUSED. Otherwise, if we haven't queued any skbs
+ * to other and its full, we will hang waiting for POLLOUT.
+ */
+ if (unix_recvq_full(other) && !sock_flag(other, SOCK_DEAD))
return 1;
if (connected)
--
1.9.1
^ permalink raw reply related
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