* RE: [virtio-dev] Re: [RFC] vhost: introduce mdev based hardware vhost backend
From: Tian, Kevin @ 2018-04-11 1:38 UTC (permalink / raw)
To: Liang, Cunming, Michael S. Tsirkin
Cc: Duyck, Alexander H, virtio-dev@lists.oasis-open.org,
kvm@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, Wang, Xiao W,
ddutile@redhat.com, Tan, Jianfeng, Wang, Zhihong, Paolo Bonzini
In-Reply-To: <D0158A423229094DA7ABF71CF2FA0DA34E9226AF@SHSMSX104.ccr.corp.intel.com>
> From: Liang, Cunming
> Sent: Tuesday, April 10, 2018 10:24 PM
>
[...]
> >
> > As others said, we do not need to go overeboard. A couple of small
> vendor-
> > specific quirks in qemu isn't a big deal.
>
> It's quite challenge to identify it's small or not, there's no uniform metric.
>
> It's only dependent on QEMU itself, that's the obvious benefit. Tradeoff is
> to build the entire device driver. We don't object to do that in QEMU, but
> wanna make sure to understand the boundary size clearly.
>
It might be helpful if you can post some sample code using proposed
framework - then people have a clear feeling about what size is talked
about here and whether it falls into the concept of 'small quirks'.
Thanks
Kevin
^ permalink raw reply
* [PATCH 2/2] staging: irda: Replace mdelay with usleep_range in irda_usb_probe
From: Jia-Ju Bai @ 2018-04-11 1:33 UTC (permalink / raw)
To: samuel, gregkh, davem, johan, arvind.yadav.cs
Cc: netdev, devel, linux-kernel, Jia-Ju Bai
irda_usb_probe() is never called in atomic context.
irda_usb_probe() is only set as ".probe" in struct usb_driver.
Despite never getting called from atomic context, irda_usb_probe()
calls mdelay() to busily wait.
This is not necessary and can be replaced with usleep_range() to
avoid busy waiting.
This is found by a static analysis tool named DCNS written by myself.
And I also manually check it.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
drivers/staging/irda/drivers/irda-usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
index 723e49b..6ff5b08 100644
--- a/drivers/staging/irda/drivers/irda-usb.c
+++ b/drivers/staging/irda/drivers/irda-usb.c
@@ -1710,7 +1710,7 @@ static int irda_usb_probe(struct usb_interface *intf,
pr_debug("usb_control_msg failed %d\n", ret);
goto err_out_3;
} else {
- mdelay(10);
+ usleep_range(10000, 11000);
}
}
--
1.9.1
^ permalink raw reply related
* [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload
From: Jia-Ju Bai @ 2018-04-11 1:29 UTC (permalink / raw)
To: samuel, gregkh, davem, johan, arvind.yadav.cs
Cc: netdev, devel, linux-kernel, Jia-Ju Bai
stir421x_fw_upload() is never called in atomic context.
The call chain ending up at stir421x_fw_upload() is:
[1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
irda_usb_probe() is set as ".probe" in struct usb_driver.
This function is not called in atomic context.
Despite never getting called from atomic context, stir421x_fw_upload()
calls mdelay() to busily wait.
This is not necessary and can be replaced with usleep_range() to
avoid busy waiting.
This is found by a static analysis tool named DCNS written by myself.
And I also manually check it.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
drivers/staging/irda/drivers/irda-usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
index 723e49b..c6c8c2c 100644
--- a/drivers/staging/irda/drivers/irda-usb.c
+++ b/drivers/staging/irda/drivers/irda-usb.c
@@ -1050,7 +1050,7 @@ static int stir421x_fw_upload(struct irda_usb_cb *self,
if (ret < 0)
break;
- mdelay(10);
+ usleep_range(10000, 11000);
}
kfree(patch_block);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v2 0/2] vhost: fix vhost_vq_access_ok() log check
From: Stefan Hajnoczi @ 2018-04-11 1:21 UTC (permalink / raw)
To: David Miller
Cc: jasowang, virtualization, syzkaller-bugs, mst, torvalds, kvm,
linux-kernel, netdev
In-Reply-To: <20180410.105043.1798364865868187298.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1182 bytes --]
On Tue, Apr 10, 2018 at 10:50:43AM -0400, David Miller wrote:
> From: Jason Wang <jasowang@redhat.com>
> Date: Tue, 10 Apr 2018 14:40:10 +0800
>
> > On 2018年04月10日 13:26, Stefan Hajnoczi wrote:
> >> v2:
> >> * Rewrote the conditional to make the vq access check clearer [Linus]
> >> * Added Patch 2 to make the return type consistent and harder to misuse
> >> * [Linus]
> >>
> >> The first patch fixes the vhost virtqueue access check which was
> >> recently
> >> broken. The second patch replaces the int return type with bool to
> >> prevent
> >> future bugs.
> >>
> >> Stefan Hajnoczi (2):
> >> vhost: fix vhost_vq_access_ok() log check
> >> vhost: return bool from *_access_ok() functions
> >>
> >> drivers/vhost/vhost.h | 4 +--
> >> drivers/vhost/vhost.c | 70
> >> ++++++++++++++++++++++++++-------------------------
> >> 2 files changed, 38 insertions(+), 36 deletions(-)
> >>
> >
> > Acked-by: Jason Wang <jasowang@redhat.com>
>
> This sereis doesn't apply cleanly to the net tree, please respin
> and add the appropriate Acks and Fixes: tags that Michael and Jason
> have provided.
Sorry, will fix.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
From: Michael S. Tsirkin @ 2018-04-11 1:21 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Sridhar Samudrala, davem, netdev, virtualization, virtio-dev,
jesse.brandeburg, alexander.h.duyck, kubakici, jasowang,
loseweigh, jiri
In-Reply-To: <20180410142608.50f15b45@xeon-e3>
On Tue, Apr 10, 2018 at 02:26:08PM -0700, Stephen Hemminger wrote:
> On Tue, 10 Apr 2018 11:59:50 -0700
> Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
>
> > Use the registration/notification framework supported by the generic
> > bypass infrastructure.
> >
> > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> > ---
>
> Thanks for doing this. Your current version has couple show stopper
> issues.
>
> First, the slave device is instantly taking over the slave.
> This doesn't allow udev/systemd to do its device rename of the slave
> device. Netvsc uses a delayed work to workaround this.
>
> Secondly, the select queue needs to call queue selection in VF.
> The bonding/teaming logic doesn't work well for UDP flows.
> Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF")
> fixed this performance problem.
>
> Lastly, more indirection is bad in current climate.
Well right now netvsc does an indirect call to the PT device,
does it not? If you really want max performance when PT
is in use you need to do the reverse and have PT forward to netvsc.
> I am not completely adverse to this but it needs to be fast, simple
> and completely transparent.
^ permalink raw reply
* [PATCHv2] mISDN: Remove VLAs
From: Laura Abbott @ 2018-04-11 1:04 UTC (permalink / raw)
To: Karsten Keil, David S. Miller
Cc: Laura Abbott, Kees Cook, netdev, linux-kernel, kernel-hardening
There's an ongoing effort to remove VLAs[1] from the kernel to eventually
turn on -Wvla. Remove the VLAs from the mISDN code by switching to using
kstrdup in one place and using an upper bound in another.
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v2: Switch to a tighter upper bound so we are allocating a more
reasonable amount on the stack (300). This is based on previous checks
against this value.
---
drivers/isdn/mISDN/dsp_hwec.c | 8 +++++---
drivers/isdn/mISDN/l1oip_core.c | 14 +++++++++++---
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/isdn/mISDN/dsp_hwec.c b/drivers/isdn/mISDN/dsp_hwec.c
index a6e87076acc2..5336bbdbfdc5 100644
--- a/drivers/isdn/mISDN/dsp_hwec.c
+++ b/drivers/isdn/mISDN/dsp_hwec.c
@@ -68,12 +68,12 @@ void dsp_hwec_enable(struct dsp *dsp, const char *arg)
goto _do;
{
- char _dup[len + 1];
char *dup, *tok, *name, *val;
int tmp;
- strcpy(_dup, arg);
- dup = _dup;
+ dup = kstrdup(arg, GFP_ATOMIC);
+ if (!dup)
+ return;
while ((tok = strsep(&dup, ","))) {
if (!strlen(tok))
@@ -89,6 +89,8 @@ void dsp_hwec_enable(struct dsp *dsp, const char *arg)
deftaps = tmp;
}
}
+
+ kfree(dup);
}
_do:
diff --git a/drivers/isdn/mISDN/l1oip_core.c b/drivers/isdn/mISDN/l1oip_core.c
index 21d50e4cc5e1..b05022f94f18 100644
--- a/drivers/isdn/mISDN/l1oip_core.c
+++ b/drivers/isdn/mISDN/l1oip_core.c
@@ -279,7 +279,7 @@ l1oip_socket_send(struct l1oip *hc, u8 localcodec, u8 channel, u32 chanmask,
u16 timebase, u8 *buf, int len)
{
u8 *p;
- u8 frame[len + 32];
+ u8 frame[MAX_DFRAME_LEN_L1 + 32];
struct socket *socket = NULL;
if (debug & DEBUG_L1OIP_MSG)
@@ -902,7 +902,11 @@ handle_dmsg(struct mISDNchannel *ch, struct sk_buff *skb)
p = skb->data;
l = skb->len;
while (l) {
- ll = (l < L1OIP_MAX_PERFRAME) ? l : L1OIP_MAX_PERFRAME;
+ /*
+ * This is technically bounded by L1OIP_MAX_PERFRAME but
+ * MAX_DFRAME_LEN_L1 < L1OIP_MAX_PERFRAME
+ */
+ ll = (l < MAX_DFRAME_LEN_L1) ? l : MAX_DFRAME_LEN_L1;
l1oip_socket_send(hc, 0, dch->slot, 0,
hc->chan[dch->slot].tx_counter++, p, ll);
p += ll;
@@ -1140,7 +1144,11 @@ handle_bmsg(struct mISDNchannel *ch, struct sk_buff *skb)
p = skb->data;
l = skb->len;
while (l) {
- ll = (l < L1OIP_MAX_PERFRAME) ? l : L1OIP_MAX_PERFRAME;
+ /*
+ * This is technically bounded by L1OIP_MAX_PERFRAME but
+ * MAX_DFRAME_LEN_L1 < L1OIP_MAX_PERFRAME
+ */
+ ll = (l < MAX_DFRAME_LEN_L1) ? l : MAX_DFRAME_LEN_L1;
l1oip_socket_send(hc, hc->codec, bch->slot, 0,
hc->chan[bch->slot].tx_counter, p, ll);
hc->chan[bch->slot].tx_counter += ll;
--
2.14.3
^ permalink raw reply related
* [PATCH] net/tls: Remove VLA usage
From: Kees Cook @ 2018-04-11 0:52 UTC (permalink / raw)
To: Dave Watson
Cc: linux-kernel, netdev, Ilya Lesokhin, Aviad Yehezkel,
David S. Miller
In the quest to remove VLAs from the kernel[1], this replaces the VLA
size with the only possible size used in the code, and adds a mechanism
to double-check future IV sizes.
[1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
Signed-off-by: Kees Cook <keescook@chromium.org>
---
net/tls/tls_sw.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 4dc766b03f00..71e79597f940 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -41,6 +41,8 @@
#include <net/strparser.h>
#include <net/tls.h>
+#define MAX_IV_SIZE TLS_CIPHER_AES_GCM_128_IV_SIZE
+
static int tls_do_decryption(struct sock *sk,
struct scatterlist *sgin,
struct scatterlist *sgout,
@@ -673,7 +675,7 @@ static int decrypt_skb(struct sock *sk, struct sk_buff *skb,
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
- char iv[TLS_CIPHER_AES_GCM_128_SALT_SIZE + tls_ctx->rx.iv_size];
+ char iv[TLS_CIPHER_AES_GCM_128_SALT_SIZE + MAX_IV_SIZE];
struct scatterlist sgin_arr[MAX_SKB_FRAGS + 2];
struct scatterlist *sgin = &sgin_arr[0];
struct strp_msg *rxm = strp_msg(skb);
@@ -1094,6 +1096,12 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
goto free_priv;
}
+ /* Sanity-check the IV size for stack allocations. */
+ if (iv_size > MAX_IV_SIZE) {
+ rc = -EINVAL;
+ goto free_priv;
+ }
+
cctx->prepend_size = TLS_HEADER_SIZE + nonce_size;
cctx->tag_size = tag_size;
cctx->overhead_size = cctx->prepend_size + cctx->tag_size;
--
2.7.4
--
Kees Cook
Pixel Security
^ permalink raw reply related
* Re: [PATCH] make net_gso_ok return false when gso_type is zero(invalid)
From: Wenhua Shi @ 2018-04-11 0:51 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: David Miller, netdev, linux-kernel
In-Reply-To: <20180410163216.GB3661@localhost.localdomain>
2018-04-10 18:32 GMT+02:00 Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>:
> On Sun, Apr 08, 2018 at 08:41:21PM +0200, Wenhua Shi wrote:
>> 2018-04-08 18:51 GMT+02:00 David Miller <davem@davemloft.net>:
>> >
>> > From: Wenhua Shi <march511@gmail.com>
>> > Date: Fri, 6 Apr 2018 03:43:39 +0200
>> >
>> > > Signed-off-by: Wenhua Shi <march511@gmail.com>
>> >
>> > This precondition should be made impossible instead of having to do
>> > an extra check everywhere that this helper is invoked, many of which
>> > are in fast paths.
>>
>> I believe the precondition you said is quite true. In my situation, I
>> have to disable GSO for some packet and I notice that it leads to a
>> worse performance (slower than 1Mbps, was almost 800Mbps).
>>
>> Here's the hook I use on debian 9.4, kernel version 4.9:
>
> There is quite a distance between 4.9 and net/net-next. Did you test
> on a more recent kernel too?
>
> Note that TCP stack now works with GSO being always on.
> 0a6b2a1dc2a2 ("tcp: switch to GSO being always on")
>
I've tried testing on the Fedora rawhide channel. The kernel version
is 4.17.0. Detail information is attached.
Without the hook
[root@fedora-s-1vcpu-1gb-sfo1-01 testing]# iperf -c
myanothernormalmachine -d
------------------------------------------------------------
Server listening on TCP port 5001
TCP window size: 85.3 KByte (default)
------------------------------------------------------------
------------------------------------------------------------
Client connecting to myanothernormalmachine, TCP port 5001
TCP window size: 85.0 KByte (default)
------------------------------------------------------------
[ 3] local 107.170.240.XXX port 44692 connected with
104.131.148.XXX port 5001
[ 5] local 107.170.240.XXX port 5001 connected with
104.131.148.XXX port 53978
[ ID] Interval Transfer Bandwidth
[ 3] 0.0-10.0 sec 1.04 GBytes 892 Mbits/sec
[ 5] 0.0-10.0 sec 757 MBytes 638 Mbits/sec
With the hook
[root@fedora-s-1vcpu-1gb-sfo1-01 testing]# iperf -c
myanothernormalmachine -d
------------------------------------------------------------
Server listening on TCP port 5001
TCP window size: 85.3 KByte (default)
------------------------------------------------------------
------------------------------------------------------------
Client connecting to myanothernormalmachine, TCP port 5001
TCP window size: 85.0 KByte (default)
------------------------------------------------------------
[ 3] local 107.170.240.XXX port 44694 connected with
104.131.148.XXX port 5001
[ 5] local 107.170.240.XXX port 5001 connected with
104.131.148.XXX port 53980
[ ID] Interval Transfer Bandwidth
[ 5] 0.0-10.0 sec 1.04 GBytes 894 Mbits/sec
[ 3] 0.0-13.5 sec 170 KBytes 103 Kbits/sec
Kernel
[root@fedora-s-1vcpu-1gb-sfo1-01 testing]# uname -a
Linux fedora-s-1vcpu-1gb-sfo1-01.localdomain
4.17.0-0.rc0.git5.2.fc29.x86_64 #1 SMP Mon Apr 9 17:16:30 UTC 2018
x86_64 x86_64 x86_64 GNU/Linux
Hook Source Code
[root@fedora-s-1vcpu-1gb-sfo1-01 testing]# cat testing.c
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/netfilter.h>
#include <linux/netfilter_ipv4.h>
#include <linux/netfilter_ipv6.h>
#include <linux/skbuff.h>
#include <linux/tcp.h>
#include <linux/ip.h>
unsigned int hook_outgoing(
void * priv,
struct sk_buff * skb,
const struct nf_hook_state * state)
{
printk(KERN_INFO "Hook working...\n");
/* for some reason I have to disable GSO */
skb_gso_reset(skb);
/* The following won't work any more. */
// skb->sk->sk_gso_type = ~0;
return NF_ACCEPT;
}
static struct nf_hook_ops hook =
{
.hook = hook_outgoing,
.pf = PF_INET,
.hooknum = NF_INET_POST_ROUTING,
.priority = NF_IP_PRI_LAST,
};
static int __init init_testing(void)
{
nf_register_net_hook(&init_net, &hook);
return 0;
}
static void __exit exit_testing(void)
{
nf_unregister_net_hook(&init_net, &hook);
}
MODULE_LICENSE("GPL");
module_init(init_testing);
module_exit(exit_testing);
It turns out the problem exists and my previous bypassing trick is not
working any more. I'm now testing whether the patch is working for the
latest net-next branch.
^ permalink raw reply
* [PATCH] netfilter: ftp helper: Support \n and \r terminators for PORT
From: Scott Parlane @ 2018-04-11 0:11 UTC (permalink / raw)
To: pablo, kadlec, fw, netfilter-devel, coreteam, netdev; +Cc: Scott Parlane
The current code requires \r as the terminator directly
after the last digit, and RFC959 indicates CRLF as the
the terminator.
However, we have seen in the wild a client sending
packets with only \n
Signed-off-by: Scott Parlane <scott.parlane@alliedtelesis.co.nz>
---
net/netfilter/nf_conntrack_ftp.c | 40 ++++++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
index b666959f17..e15bf351f6 100644
--- a/net/netfilter/nf_conntrack_ftp.c
+++ b/net/netfilter/nf_conntrack_ftp.c
@@ -56,21 +56,23 @@ unsigned int (*nf_nat_ftp_hook)(struct sk_buff *skb,
EXPORT_SYMBOL_GPL(nf_nat_ftp_hook);
static int try_rfc959(const char *, size_t, struct nf_conntrack_man *,
- char, unsigned int *);
+ char, char, unsigned int *);
static int try_rfc1123(const char *, size_t, struct nf_conntrack_man *,
- char, unsigned int *);
+ char, char, unsigned int *);
static int try_eprt(const char *, size_t, struct nf_conntrack_man *,
- char, unsigned int *);
+ char, char, unsigned int *);
static int try_epsv_response(const char *, size_t, struct nf_conntrack_man *,
- char, unsigned int *);
+ char, char, unsigned int *);
static struct ftp_search {
const char *pattern;
size_t plen;
char skip;
char term;
+ char alt_term;
enum nf_ct_ftp_type ftptype;
- int (*getnum)(const char *, size_t, struct nf_conntrack_man *, char, unsigned int *);
+ int (*getnum)(const char *, size_t, struct nf_conntrack_man *, char,
+ char, unsigned int *);
} search[IP_CT_DIR_MAX][2] = {
[IP_CT_DIR_ORIGINAL] = {
{
@@ -78,6 +80,7 @@ static struct ftp_search {
.plen = sizeof("PORT") - 1,
.skip = ' ',
.term = '\r',
+ .alt_term = '\n',
.ftptype = NF_CT_FTP_PORT,
.getnum = try_rfc959,
},
@@ -119,7 +122,7 @@ get_ipv6_addr(const char *src, size_t dlen, struct in6_addr *dst, u_int8_t term)
}
static int try_number(const char *data, size_t dlen, u_int32_t array[],
- int array_size, char sep, char term)
+ int array_size, char sep, char term, char alt_term)
{
u_int32_t i, len;
@@ -136,7 +139,9 @@ static int try_number(const char *data, size_t dlen, u_int32_t array[],
/* Unexpected character; true if it's the
terminator (or we don't care about one)
and we're finished. */
- if ((*data == term || !term) && i == array_size - 1)
+ if ((*data == term || (alt_term &&
+ *data == alt_term) || !term) &&
+ i == array_size - 1)
return len;
pr_debug("Char %u (got %u nums) `%u' unexpected\n",
@@ -152,12 +157,12 @@ static int try_number(const char *data, size_t dlen, u_int32_t array[],
/* Returns 0, or length of numbers: 192,168,1,1,5,6 */
static int try_rfc959(const char *data, size_t dlen,
struct nf_conntrack_man *cmd, char term,
- unsigned int *offset)
+ char alt_term, unsigned int *offset)
{
int length;
u_int32_t array[6];
- length = try_number(data, dlen, array, 6, ',', term);
+ length = try_number(data, dlen, array, 6, ',', term, alt_term);
if (length == 0)
return 0;
@@ -179,7 +184,7 @@ static int try_rfc959(const char *data, size_t dlen,
*/
static int try_rfc1123(const char *data, size_t dlen,
struct nf_conntrack_man *cmd, char term,
- unsigned int *offset)
+ char alt_term, unsigned int *offset)
{
int i;
for (i = 0; i < dlen; i++)
@@ -191,7 +196,7 @@ static int try_rfc1123(const char *data, size_t dlen,
*offset += i;
- return try_rfc959(data + i, dlen - i, cmd, 0, offset);
+ return try_rfc959(data + i, dlen - i, cmd, 0, 0, offset);
}
/* Grab port: number up to delimiter */
@@ -222,7 +227,7 @@ static int get_port(const char *data, int start, size_t dlen, char delim,
/* Returns 0, or length of numbers: |1|132.235.1.2|6275| or |2|3ffe::1|6275| */
static int try_eprt(const char *data, size_t dlen, struct nf_conntrack_man *cmd,
- char term, unsigned int *offset)
+ char term, char alt_term, unsigned int *offset)
{
char delim;
int length;
@@ -251,7 +256,8 @@ static int try_eprt(const char *data, size_t dlen, struct nf_conntrack_man *cmd,
u_int32_t array[4];
/* Now we have IP address. */
- length = try_number(data + 3, dlen - 3, array, 4, '.', delim);
+ length = try_number(data + 3, dlen - 3, array, 4, '.', delim,
+ 0);
if (length != 0)
cmd->u3.ip = htonl((array[0] << 24) | (array[1] << 16)
| (array[2] << 8) | array[3]);
@@ -271,7 +277,7 @@ static int try_eprt(const char *data, size_t dlen, struct nf_conntrack_man *cmd,
/* Returns 0, or length of numbers: |||6446| */
static int try_epsv_response(const char *data, size_t dlen,
struct nf_conntrack_man *cmd, char term,
- unsigned int *offset)
+ char alt_term, unsigned int *offset)
{
char delim;
@@ -289,12 +295,13 @@ static int try_epsv_response(const char *data, size_t dlen,
static int find_pattern(const char *data, size_t dlen,
const char *pattern, size_t plen,
char skip, char term,
+ char alt_term,
unsigned int *numoff,
unsigned int *numlen,
struct nf_conntrack_man *cmd,
int (*getnum)(const char *, size_t,
struct nf_conntrack_man *, char,
- unsigned int *))
+ char, unsigned int *))
{
size_t i = plen;
@@ -337,7 +344,7 @@ static int find_pattern(const char *data, size_t dlen,
pr_debug("Skipped up to `%c'!\n", skip);
*numoff = i;
- *numlen = getnum(data + i, dlen - i, cmd, term, numoff);
+ *numlen = getnum(data + i, dlen - i, cmd, term, alt_term, numoff);
if (!*numlen)
return -1;
@@ -461,6 +468,7 @@ skip_nl_seq:
search[dir][i].plen,
search[dir][i].skip,
search[dir][i].term,
+ search[dir][i].alt_term,
&matchoff, &matchlen,
&cmd,
search[dir][i].getnum);
--
2.16.1
^ permalink raw reply related
* Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
From: Stephen Hemminger @ 2018-04-10 23:59 UTC (permalink / raw)
To: Siwei Liu
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Michael S. Tsirkin,
Jakub Kicinski, Sridhar Samudrala, virtualization, Netdev,
David Miller
In-Reply-To: <CADGSJ22rVsC0TDTd6OKVnwbx0ExoQ8xWXBMumKB-OFH4sX=yaQ@mail.gmail.com>
On Tue, 10 Apr 2018 16:44:47 -0700
Siwei Liu <loseweigh@gmail.com> wrote:
> On Tue, Apr 10, 2018 at 4:28 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Apr 10, 2018 at 02:26:08PM -0700, Stephen Hemminger wrote:
> >> On Tue, 10 Apr 2018 11:59:50 -0700
> >> Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
> >>
> >> > Use the registration/notification framework supported by the generic
> >> > bypass infrastructure.
> >> >
> >> > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> >> > ---
> >>
> >> Thanks for doing this. Your current version has couple show stopper
> >> issues.
> >>
> >> First, the slave device is instantly taking over the slave.
> >> This doesn't allow udev/systemd to do its device rename of the slave
> >> device. Netvsc uses a delayed work to workaround this.
> >
> > Interesting. Does this mean udev must act within a specific time window
> > then?
>
> Sighs, lots of hacks. Why propgating this from driver to a common
> module. We really need a clean solution.
>
I had a patch to wait for udev to do the rename and go from there
but davem rejected it.
^ permalink raw reply
* Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
From: Siwei Liu @ 2018-04-10 23:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Stephen Hemminger, Sridhar Samudrala, David Miller, Netdev,
virtualization, virtio-dev, Brandeburg, Jesse, Alexander Duyck,
Jakub Kicinski, Jason Wang, Jiri Pirko
In-Reply-To: <20180411022807-mutt-send-email-mst@kernel.org>
On Tue, Apr 10, 2018 at 4:28 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Apr 10, 2018 at 02:26:08PM -0700, Stephen Hemminger wrote:
>> On Tue, 10 Apr 2018 11:59:50 -0700
>> Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
>>
>> > Use the registration/notification framework supported by the generic
>> > bypass infrastructure.
>> >
>> > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>> > ---
>>
>> Thanks for doing this. Your current version has couple show stopper
>> issues.
>>
>> First, the slave device is instantly taking over the slave.
>> This doesn't allow udev/systemd to do its device rename of the slave
>> device. Netvsc uses a delayed work to workaround this.
>
> Interesting. Does this mean udev must act within a specific time window
> then?
Sighs, lots of hacks. Why propgating this from driver to a common
module. We really need a clean solution.
-Siwei
>
>> Secondly, the select queue needs to call queue selection in VF.
>> The bonding/teaming logic doesn't work well for UDP flows.
>> Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF")
>> fixed this performance problem.
>>
>> Lastly, more indirection is bad in current climate.
>>
>> I am not completely adverse to this but it needs to be fast, simple
>> and completely transparent.
^ permalink raw reply
* Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
From: Michael S. Tsirkin @ 2018-04-10 23:28 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Sridhar Samudrala, davem, netdev, virtualization, virtio-dev,
jesse.brandeburg, alexander.h.duyck, kubakici, jasowang,
loseweigh, jiri
In-Reply-To: <20180410142608.50f15b45@xeon-e3>
On Tue, Apr 10, 2018 at 02:26:08PM -0700, Stephen Hemminger wrote:
> On Tue, 10 Apr 2018 11:59:50 -0700
> Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
>
> > Use the registration/notification framework supported by the generic
> > bypass infrastructure.
> >
> > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> > ---
>
> Thanks for doing this. Your current version has couple show stopper
> issues.
>
> First, the slave device is instantly taking over the slave.
> This doesn't allow udev/systemd to do its device rename of the slave
> device. Netvsc uses a delayed work to workaround this.
Interesting. Does this mean udev must act within a specific time window
then?
> Secondly, the select queue needs to call queue selection in VF.
> The bonding/teaming logic doesn't work well for UDP flows.
> Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF")
> fixed this performance problem.
>
> Lastly, more indirection is bad in current climate.
>
> I am not completely adverse to this but it needs to be fast, simple
> and completely transparent.
^ permalink raw reply
* Re: [PATCH] net/mlx5: remove some extraneous spaces in indentations
From: Jason Gunthorpe @ 2018-04-10 23:27 UTC (permalink / raw)
To: Colin Ian King
Cc: Saeed Mahameed, Matan Barak, Leon Romanovsky, netdev, linux-rdma,
kernel-janitors, linux-kernel
In-Reply-To: <20180410232254.mg7stooh5tytv6m2@ziepe.ca>
On Tue, Apr 10, 2018 at 05:22:54PM -0600, Jason Gunthorpe wrote:
> On Mon, Apr 09, 2018 at 01:43:36PM +0100, Colin Ian King wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > There are several lines where there is an extraneous space causing
> > indentation misalignment. Remove them.
> >
> > Cleans up Cocconelle warnings:
> >
> > ./drivers/net/ethernet/mellanox/mlx5/core/qp.c:409:3-18: code aligned
> > with following code on line 410
> > ./drivers/net/ethernet/mellanox/mlx5/core/qp.c:415:3-18: code aligned
> > with following code on line 416
> > ./drivers/net/ethernet/mellanox/mlx5/core/qp.c:421:3-18: code aligned
> > with following code on line 422
> >
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > drivers/net/ethernet/mellanox/mlx5/core/qp.c | 18 +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)
>
> applied to for-next thanks
Oh wait, this is for netdev, not rdma sorry. Never mind, DaveM should
pick it up.
Jason
^ permalink raw reply
* Re: [PATCH] net/mlx5: remove some extraneous spaces in indentations
From: Jason Gunthorpe @ 2018-04-10 23:22 UTC (permalink / raw)
To: Colin Ian King
Cc: Saeed Mahameed, Matan Barak, Leon Romanovsky, netdev, linux-rdma,
kernel-janitors, linux-kernel
In-Reply-To: <20180409124336.29274-1-colin.king@canonical.com>
On Mon, Apr 09, 2018 at 01:43:36PM +0100, Colin Ian King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There are several lines where there is an extraneous space causing
> indentation misalignment. Remove them.
>
> Cleans up Cocconelle warnings:
>
> ./drivers/net/ethernet/mellanox/mlx5/core/qp.c:409:3-18: code aligned
> with following code on line 410
> ./drivers/net/ethernet/mellanox/mlx5/core/qp.c:415:3-18: code aligned
> with following code on line 416
> ./drivers/net/ethernet/mellanox/mlx5/core/qp.c:421:3-18: code aligned
> with following code on line 422
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/qp.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
applied to for-next thanks
Jason
^ permalink raw reply
* Re: [PATCH bpf v4] bpf/tracing: fix a deadlock in perf_event_detach_bpf_prog
From: Daniel Borkmann @ 2018-04-10 23:14 UTC (permalink / raw)
To: Yonghong Song, ast, netdev; +Cc: kernel-team
In-Reply-To: <20180410163732.2818747-1-yhs@fb.com>
On 04/10/2018 06:37 PM, Yonghong Song wrote:
> syzbot reported a possible deadlock in perf_event_detach_bpf_prog.
> The error details:
> ======================================================
> WARNING: possible circular locking dependency detected
> 4.16.0-rc7+ #3 Not tainted
> ------------------------------------------------------
> syz-executor7/24531 is trying to acquire lock:
> (bpf_event_mutex){+.+.}, at: [<000000008a849b07>] perf_event_detach_bpf_prog+0x92/0x3d0 kernel/trace/bpf_trace.c:854
>
> but task is already holding lock:
> (&mm->mmap_sem){++++}, at: [<0000000038768f87>] vm_mmap_pgoff+0x198/0x280 mm/util.c:353
>
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
[...]
>
> The bug is introduced by Commit f371b304f12e ("bpf/tracing: allow
> user space to query prog array on the same tp") where copy_to_user,
> which requires mm->mmap_sem, is called inside bpf_event_mutex lock.
> At the same time, during perf_event file descriptor close,
> mm->mmap_sem is held first and then subsequent
> perf_event_detach_bpf_prog needs bpf_event_mutex lock.
> Such a senario caused a deadlock.
>
> As suggested by Daniel, moving copy_to_user out of the
> bpf_event_mutex lock should fix the problem.
>
> Fixes: f371b304f12e ("bpf/tracing: allow user space to query prog array on the same tp")
> Reported-by: syzbot+dc5ca0e4c9bfafaf2bae@syzkaller.appspotmail.com
> Signed-off-by: Yonghong Song <yhs@fb.com>
Applied to bpf tree, thanks Yonghong!
^ permalink raw reply
* Re: [PATCH] selftests: bpf: update .gitignore with missing generated files
From: Daniel Borkmann @ 2018-04-10 23:14 UTC (permalink / raw)
To: Anders Roxell, ast, shuah; +Cc: netdev, linux-kernel, linux-kselftest
In-Reply-To: <20180410122421.25393-1-anders.roxell@linaro.org>
On 04/10/2018 02:24 PM, Anders Roxell wrote:
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
Applied to bpf tree, thanks Anders!
^ permalink raw reply
* Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
From: Samudrala, Sridhar @ 2018-04-10 22:56 UTC (permalink / raw)
To: Stephen Hemminger
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, davem
In-Reply-To: <20180410142608.50f15b45@xeon-e3>
On 4/10/2018 2:26 PM, Stephen Hemminger wrote:
> On Tue, 10 Apr 2018 11:59:50 -0700
> Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
>
>> Use the registration/notification framework supported by the generic
>> bypass infrastructure.
>>
>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>> ---
> Thanks for doing this. Your current version has couple show stopper
> issues.
>
> First, the slave device is instantly taking over the slave.
> This doesn't allow udev/systemd to do its device rename of the slave
> device. Netvsc uses a delayed work to workaround this.
OK. I guess you are referring to the dev_set_mtu() and dev_open() calls that are
made in bypass_slave_register() and you want to defer them to be done after
a delay. I could avoid these calls in case of netvsc based on bypass_ops.
>
> Secondly, the select queue needs to call queue selection in VF.
> The bonding/teaming logic doesn't work well for UDP flows.
> Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF")
> fixed this performance problem.
netvsc should not be using bypass_select_queue() as that ndo op gets used
only with 3-netdev model.
Anyway, will look into updating bypass_select_queue() based on your fix.
>
> Lastly, more indirection is bad in current climate.
>
> I am not completely adverse to this but it needs to be fast, simple
> and completely transparent.
Not sure we can avoid this indirection if we want to commonize the code, but use
different models for virtio-net and netvsc.
On the other hand, these patches avoid calls to get_netvsc_bymac() and
get_netvsc_by_ref() that go through all the devices for all the netdev events.
netvsc lookups should be much faster.
Thanks
Sridhar
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC bpf-next v2 3/8] bpf: add documentation for eBPF helpers (12-22)
From: Alexei Starovoitov @ 2018-04-10 22:43 UTC (permalink / raw)
To: Quentin Monnet; +Cc: daniel, ast, netdev, oss-drivers, linux-doc, linux-man
In-Reply-To: <20180410144157.4831-4-quentin.monnet@netronome.com>
On Tue, Apr 10, 2018 at 03:41:52PM +0100, Quentin Monnet wrote:
> Add documentation for eBPF helper functions to bpf.h user header file.
> This documentation can be parsed with the Python script provided in
> another commit of the patch series, in order to provide a RST document
> that can later be converted into a man page.
>
> The objective is to make the documentation easily understandable and
> accessible to all eBPF developers, including beginners.
>
> This patch contains descriptions for the following helper functions, all
> writter by Alexei:
>
> - bpf_get_current_pid_tgid()
> - bpf_get_current_uid_gid()
> - bpf_get_current_comm()
> - bpf_skb_vlan_push()
> - bpf_skb_vlan_pop()
> - bpf_skb_get_tunnel_key()
> - bpf_skb_set_tunnel_key()
> - bpf_redirect()
> - bpf_perf_event_output()
> - bpf_get_stackid()
> - bpf_get_current_task()
>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> ---
> include/uapi/linux/bpf.h | 237 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 237 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 2bc653a3a20f..f3ea8824efbc 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -580,6 +580,243 @@ union bpf_attr {
> * performed again.
> * Return
> * 0 on success, or a negative error in case of failure.
> + *
> + * u64 bpf_get_current_pid_tgid(void)
> + * Return
> + * A 64-bit integer containing the current tgid and pid, and
> + * created as such:
> + * *current_task*\ **->tgid << 32 \|**
> + * *current_task*\ **->pid**.
> + *
> + * u64 bpf_get_current_uid_gid(void)
> + * Return
> + * A 64-bit integer containing the current GID and UID, and
> + * created as such: *current_gid* **<< 32 \|** *current_uid*.
> + *
> + * int bpf_get_current_comm(char *buf, u32 size_of_buf)
> + * Description
> + * Copy the **comm** attribute of the current task into *buf* of
> + * *size_of_buf*. The **comm** attribute contains the name of
> + * the executable (excluding the path) for the current task. The
> + * *size_of_buf* must be strictly positive. On success, the
that reminds me that we probably should relax it to ARG_CONST_SIZE_OR_ZERO.
The programs won't be passing an actual zero into it, but it helps
a lot to tell verifier that zero is also valid, since programs
become much simpler.
> + * helper makes sure that the *buf* is NUL-terminated. On failure,
> + * it is filled with zeroes.
> + * Return
> + * 0 on success, or a negative error in case of failure.
> + *
> + * int bpf_skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
> + * Description
> + * Push a *vlan_tci* (VLAN tag control information) of protocol
> + * *vlan_proto* to the packet associated to *skb*, then update
> + * the checksum. Note that if *vlan_proto* is different from
> + * **ETH_P_8021Q** and **ETH_P_8021AD**, it is considered to
> + * be **ETH_P_8021Q**.
> + *
> + * A call to this helper is susceptible to change data from the
> + * packet. Therefore, at load time, all checks on pointers
> + * previously done by the verifier are invalidated and must be
> + * performed again.
> + * Return
> + * 0 on success, or a negative error in case of failure.
> + *
> + * int bpf_skb_vlan_pop(struct sk_buff *skb)
> + * Description
> + * Pop a VLAN header from the packet associated to *skb*.
> + *
> + * A call to this helper is susceptible to change data from the
> + * packet. Therefore, at load time, all checks on pointers
> + * previously done by the verifier are invalidated and must be
> + * performed again.
> + * Return
> + * 0 on success, or a negative error in case of failure.
> + *
> + * int bpf_skb_get_tunnel_key(struct sk_buff *skb, struct bpf_tunnel_key *key, u32 size, u64 flags)
> + * Description
> + * Get tunnel metadata. This helper takes a pointer *key* to an
> + * empty **struct bpf_tunnel_key** of **size**, that will be
> + * filled with tunnel metadata for the packet associated to *skb*.
> + * The *flags* can be set to **BPF_F_TUNINFO_IPV6**, which
> + * indicates that the tunnel is based on IPv6 protocol instead of
> + * IPv4.
> + *
> + * This is typically used on the receive path to perform a lookup
> + * or a packet redirection based on the value of *key*:
above is correct, but feels a bit cryptic.
May be give more concrete example for particular tunneling protocol like gre
and say that tunnel_key.remote_ip[46] is essential part of the encap and
bpf prog will make decisions based on the contents of the encap header
where bpf_tunnel_key is a single structure that generalizes parameters of
various tunneling protocols into one struct.
> + *
> + * ::
> + *
> + * struct bpf_tunnel_key key = {};
> + * bpf_skb_get_tunnel_key(skb, &key, sizeof(key), 0);
> + * lookup or redirect based on key ...
> + *
> + * Return
> + * 0 on success, or a negative error in case of failure.
> + *
> + * int bpf_skb_set_tunnel_key(struct sk_buff *skb, struct bpf_tunnel_key *key, u32 size, u64 flags)
> + * Description
> + * Populate tunnel metadata for packet associated to *skb.* The
> + * tunnel metadata is set to the contents of *key*, of *size*. The
> + * *flags* can be set to a combination of the following values:
> + *
> + * **BPF_F_TUNINFO_IPV6**
> + * Indicate that the tunnel is based on IPv6 protocol
> + * instead of IPv4.
> + * **BPF_F_ZERO_CSUM_TX**
> + * For IPv4 packets, add a flag to tunnel metadata
> + * indicating that checksum computation should be skipped
> + * and checksum set to zeroes.
> + * **BPF_F_DONT_FRAGMENT**
> + * Add a flag to tunnel metadata indicating that the
> + * packet should not be fragmented.
> + * **BPF_F_SEQ_NUMBER**
> + * Add a flag to tunnel metadata indicating that a
> + * sequence number should be added to tunnel header before
> + * sending the packet. This flag was added for GRE
> + * encapsulation, but might be used with other protocols
> + * as well in the future.
> + *
> + * Here is a typical usage on the transmit path:
> + *
> + * ::
> + *
> + * struct bpf_tunnel_key key;
> + * populate key ...
> + * bpf_skb_set_tunnel_key(skb, &key, sizeof(key), 0);
> + * bpf_clone_redirect(skb, vxlan_dev_ifindex, 0);
> + *
> + * Return
> + * 0 on success, or a negative error in case of failure.
> + *
> + * int bpf_redirect(u32 ifindex, u64 flags)
> + * Description
> + * Redirect the packet to another net device of index *ifindex*.
> + * This helper is somewhat similar to **bpf_clone_redirect**\
> + * (), except that the packet is not cloned, which provides
> + * increased performance.
> + *
> + * For hooks other than XDP, *flags* can be set to
> + * **BPF_F_INGRESS**, which indicates the packet is to be
> + * redirected to the ingress interface instead of (by default)
> + * egress. Currently, XDP does not support any flag.
> + * Return
> + * For XDP, the helper returns **XDP_REDIRECT** on success or
> + * **XDP_ABORT** on error. For other program types, the values
> + * are **TC_ACT_REDIRECT** on success or **TC_ACT_SHOT** on
> + * error.
> + *
> + * int bpf_perf_event_output(struct pt_reg *ctx, struct bpf_map *map, u64 flags, void *data, u64 size)
> + * Description
> + * Write perf raw sample into a perf event held by *map* of type
I'd say:
Write raw *data* blob into special bpf perf event held by ...
> + * **BPF_MAP_TYPE_PERF_EVENT_ARRAY**. This perf event must
> + * have the following attributes: **PERF_SAMPLE_RAW** as
> + * **sample_type**, **PERF_TYPE_SOFTWARE** as **type**, and
> + * **PERF_COUNT_SW_BPF_OUTPUT** as **config**.
> + *
> + * The *flags* are used to indicate the index in *map* for which
> + * the value must be put, masked with **BPF_F_INDEX_MASK**.
> + * Alternatively, *flags* can be set to **BPF_F_CURRENT_CPU**
> + * to indicate that the index of the current CPU core should be
> + * used.
> + *
> + * The value to write, of *size*, is passed through eBPF stack and
> + * pointed by *data*.
> + *
> + * The context of the program *ctx* needs also be passed to the
> + * helper, and will get interpreted as a pointer to a **struct
> + * pt_reg**.
Not quite correct.
Initially bpf_perf_event_output() was only used with 'struct pt_reg *ctx',
but then later it was generalized for all other tracing prog types,
for clsact and even for XDP.
So 'ctx' can be any of the context used by these program types.
> + *
> + * On user space, a program willing to read the values needs to
> + * call **perf_event_open**\ () on the perf event (either for
> + * one or for all CPUs) and to store the file descriptor into the
> + * *map*. This must be done before the eBPF program can send data
> + * into it. An example is available in file
> + * *samples/bpf/trace_output_user.c* in the Linux kernel source
> + * tree (the eBPF program counterpart is in
> + * *samples/bpf/trace_output_kern.c*). It looks like the
> + * following snippet:
> + *
> + * ::
> + *
> + * volatile struct perf_event_mmap_page *header;
> + * struct perf_event_attr attr = {
> + * .sample_type = PERF_SAMPLE_RAW,
> + * .type = PERF_TYPE_SOFTWARE,
> + * .config = PERF_COUNT_SW_BPF_OUTPUT,
> + * };
> + * int page_size;
> + * int mmap_size;
> + * int key = 0;
> + * int pmu_fd;
> + * void *base;
> + *
> + * if (load_bpf_file(filename))
> + * return -1;
> + *
> + * pmu_fd = sys_perf_event_open(&attr,
> + * -1, // pid
> + * 0, // cpu
> + * -1, // group_fd
> + * 0);
> + *
> + * assert(pmu_fd >= 0);
> + * assert(bpf_map_update_elem(map_fd[0], &key,
> + * &pmu_fd, BPF_ANY) == 0);
> + * assert(ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0) == 0);
> + *
> + * page_size = getpagesize();
> + * mmap_size = page_size * (page_cnt + 1);
> + *
> + * base = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
> + * MAP_SHARED, fd, 0);
> + * if (base == MAP_FAILED)
> + * return -1;
> + *
> + * header = base;
I think that is too much for the man page, especially above is far from
complete example.
> + *
> + * **bpf_perf_event_output**\ () achieves better performance
> + * than **bpf_trace_printk**\ () for sharing data with user
> + * space, and is much better suitable for streaming data from eBPF
> + * programs.
> + * Return
> + * 0 on success, or a negative error in case of failure.
> + *
> + * int bpf_get_stackid(struct pt_reg *ctx, struct bpf_map *map, u64 flags)
> + * Description
> + * Walk a user or a kernel stack and return its id. To achieve
> + * this, the helper needs *ctx*, which is a pointer to the context
> + * on which the tracing program is executed, and a pointer to a
> + * *map* of type **BPF_MAP_TYPE_STACK_TRACE**.
> + *
> + * The last argument, *flags*, holds the number of stack frames to
> + * skip (from 0 to 255), masked with
> + * **BPF_F_SKIP_FIELD_MASK**. The next bits can be used to set
> + * a combination of the following flags:
> + *
> + * **BPF_F_USER_STACK**
> + * Collect a user space stack instead of a kernel stack.
> + * **BPF_F_FAST_STACK_CMP**
> + * Compare stacks by hash only.
> + * **BPF_F_REUSE_STACKID**
> + * If two different stacks hash into the same *stackid*,
> + * discard the old one.
we have an annoying bug here that we will be sending a patch to fix soon,
since right now there is no way for the program to know that stackid
got replaced.
> + *
> + * The stack id retrieved is a 32 bit long integer handle which
> + * can be further combined with other data (including other stack
> + * ids) and used as a key into maps. This can be useful for
> + * generating a variety of graphs (such as flame graphs or off-cpu
> + * graphs).
> + *
> + * For walking a stack, this helper is an improvement over
> + * **bpf_probe_read**\ (), which can be used with unrolled loops
> + * but is not efficient and consumes a lot of eBPF instructions.
> + * Instead, **bpf_get_stackid**\ () can collect up to
> + * **PERF_MAX_STACK_DEPTH** both kernel and user frames.
PERF_MAX_STACK_DEPTH is now controlled by sysctl knob.
Would be good to mention that this limit can and should be increased
for profiling long user stacks like java.
> + * Return
> + * The positive or null stack id on success, or a negative error
> + * in case of failure.
> + *
> + * u64 bpf_get_current_task(void)
> + * Return
> + * A pointer to the current task struct.
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> --
> 2.14.1
>
^ permalink raw reply
* [PATCH] ibmvnic: Define vnic_login_client_data name field as unsized array
From: Kees Cook @ 2018-04-10 22:26 UTC (permalink / raw)
To: Thomas Falcon
Cc: John Allen, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, netdev, linuxppc-dev, linux-kernel,
Daniel Micay
The "name" field of struct vnic_login_client_data is a char array of
undefined length. This should be written as "char name[]" so the compiler
can make better decisions about the field (for example, not assuming
it's a single character). This was noticed while trying to tighten the
CONFIG_FORTIFY_SOURCE checking.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/net/ethernet/ibm/ibmvnic.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index aad5658d79d5..35fbb41cd2d4 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3170,7 +3170,7 @@ static int send_version_xchg(struct ibmvnic_adapter *adapter)
struct vnic_login_client_data {
u8 type;
__be16 len;
- char name;
+ char name[];
} __packed;
static int vnic_client_data_len(struct ibmvnic_adapter *adapter)
@@ -3199,21 +3199,21 @@ static void vnic_add_client_data(struct ibmvnic_adapter *adapter,
vlcd->type = 1;
len = strlen(os_name) + 1;
vlcd->len = cpu_to_be16(len);
- strncpy(&vlcd->name, os_name, len);
- vlcd = (struct vnic_login_client_data *)((char *)&vlcd->name + len);
+ strncpy(vlcd->name, os_name, len);
+ vlcd = (struct vnic_login_client_data *)(vlcd->name + len);
/* Type 2 - LPAR name */
vlcd->type = 2;
len = strlen(utsname()->nodename) + 1;
vlcd->len = cpu_to_be16(len);
- strncpy(&vlcd->name, utsname()->nodename, len);
- vlcd = (struct vnic_login_client_data *)((char *)&vlcd->name + len);
+ strncpy(vlcd->name, utsname()->nodename, len);
+ vlcd = (struct vnic_login_client_data *)(vlcd->name + len);
/* Type 3 - device name */
vlcd->type = 3;
len = strlen(adapter->netdev->name) + 1;
vlcd->len = cpu_to_be16(len);
- strncpy(&vlcd->name, adapter->netdev->name, len);
+ strncpy(vlcd->name, adapter->netdev->name, len);
}
static int send_login(struct ibmvnic_adapter *adapter)
--
2.7.4
--
Kees Cook
Pixel Security
^ permalink raw reply related
* Re: [RFC v3 net-next 13/18] net/sched: Introduce the TBS Qdisc
From: Jesus Sanchez-Palencia @ 2018-04-10 21:24 UTC (permalink / raw)
To: Thomas Gleixner
Cc: netdev, jhs, xiyou.wangcong, jiri, vinicius.gomes, richardcochran,
anna-maria, henrik, John Stultz, levi.pearson, edumazet, willemb,
mlichvar
In-Reply-To: <alpine.DEB.2.21.1804101412420.2058@nanos.tec.linutronix.de>
Hi Thomas,
On 04/10/2018 05:37 AM, Thomas Gleixner wrote:
(...)
>>>
>>> - Simple feed through (Applications are time contraints aware and set the
>>> exact schedule). qdisc has admission control.
>>
>> This will be provided by the tbs qdisc. It will still provide a txtime sorted
>> list and hw offload, but now there will be a per-socket option that tells the
>> qdisc if the per-packet timestamp is the txtime (i.e. explicit mode, as you've
>> called it) or a deadline. The drop_if_late flag will be removed.
>>
>> When in explicit mode, packets from that socket are dequeued from the qdisc
>> during its time slice if their [(txtime - delta) < now].
>>
>>>
>>> - Deadline aware qdisc to handle e.g. A/V streams. Applications are aware
>>> of time constraints and provide the packet deadline. qdisc has admission
>>> control. This can be a simple first comes, first served scheduler or
>>> something like EDF which allows optimized utilization. The qdisc sets
>>> the TX time depending on the deadline and feeds into the root.
>>
>> This will be provided by tbs if the socket which is transmitting packets is
>> configured for deadline mode.
>
> You don't want the socket to decide that. The qdisc into which a socket
> feeds defines the mode and the qdisc rejects requests with the wrong mode.
>
> Making a qdisc doing both and let the user decide what he wants it to be is
> not really going to fly. Especially if you have different users which want
> a different mode. It's clearly distinct functionality.
Ok, so just to make sure I got this right, are you suggesting that both the
'tbs' qdisc *and* the socket (i.e. through SO_TXTIME) should have a config
parameter for specifying the txtime mode? This way if there is a mismatch,
packets from that socket are rejected by the qdisc.
(...)
>
>> Another question for this mode (but perhaps that applies to both modes) is, what
>> if the qdisc misses the deadline for *any* reason? I'm assuming it should drop
>> the packet during dequeue.
>
> There the question is how user space is notified about that issue. The
> application which queued the packet on time does rightfully assume that
> it's going to be on the wire on time.
>
> This is a violation of the overall scheduling plan, so you need to have
> a sane design to handle that.
In addition to the qdisc stats, we could look into using the socket's error
queue to notify the application about that.
>
>> Putting it all together, we end up with:
>>
>> 1) a new txtime aware qdisc, tbs, to be used per queue. Its cli will look like:
>> $ tc qdisc add (...) tbs clockid CLOCK_REALTIME delta 150000 offload sorting
>
> Why CLOCK_REALTIME? The only interesting time in a TSN network is
> CLOCK_TAI, really.
REALTIME was just an example here to show that the qdisc has to be configured
with a clockid parameter. Are you suggesting that instead both of the new qdiscs
(i.e. tbs and taprio) should always be using CLOCK_TAI implicitly?
>
>> 2) a new cmsg-interface for setting a per-packet timestamp that will be used
>> either as a txtime or as deadline by tbs (and further the NIC driver for the
>> offlaod case): SCM_TXTIME.
>>
>> 3) a new socket option: SO_TXTIME. It will be used to enable the feature for a
>> socket, and will have as parameters a clockid and a txtime mode (deadline or
>> explicit), that defines the semantics of the timestamp set on packets using
>> SCM_TXTIME.
>>
>> 4) a new #define DYNAMIC_CLOCKID 15 added to include/uapi/linux/time.h .
>
> Can you remind me why we would need that?
So there is a "clockid" that can be used for the full hw offload modes. On this
case, the txtimes are in reference to the NIC's PTP clock, and, as discussed, we
can't just use a clockid that was computed from the fd pointing to /dev/ptpX .
>
>> 5) a new schedule-aware qdisc, 'tas' or 'taprio', to be used per port. Its cli
>> will look like what was proposed for taprio (base time being an absolute timestamp).
>>
>> If we all agree with the above, we will start by closing on 1-4 asap and will
>> focus on 5 next.
>>
>> How does that sound?
>
> Backwards to be honest.
>
> You should start with the NIC facing qdisc because that's the key part of
> all this and the design might have implications on how the qdiscs which
> feed into it need to be designed.
Ok, let's just try to close on the above first.
Thanks,
Jesus
>
> Thanks,
>
> tglx
>
^ permalink raw reply
* Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
From: Stephen Hemminger @ 2018-04-10 21:26 UTC (permalink / raw)
To: Sridhar Samudrala
Cc: mst, davem, netdev, virtualization, virtio-dev, jesse.brandeburg,
alexander.h.duyck, kubakici, jasowang, loseweigh, jiri
In-Reply-To: <1523386790-12396-5-git-send-email-sridhar.samudrala@intel.com>
On Tue, 10 Apr 2018 11:59:50 -0700
Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
> Use the registration/notification framework supported by the generic
> bypass infrastructure.
>
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---
Thanks for doing this. Your current version has couple show stopper
issues.
First, the slave device is instantly taking over the slave.
This doesn't allow udev/systemd to do its device rename of the slave
device. Netvsc uses a delayed work to workaround this.
Secondly, the select queue needs to call queue selection in VF.
The bonding/teaming logic doesn't work well for UDP flows.
Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF")
fixed this performance problem.
Lastly, more indirection is bad in current climate.
I am not completely adverse to this but it needs to be fast, simple
and completely transparent.
^ permalink raw reply
* Re: [patch net] devlink: convert occ_get op to separate registration
From: Sasha Levin @ 2018-04-10 20:43 UTC (permalink / raw)
To: David Miller
Cc: jiri@resnulli.us, jiri@mellanox.com, netdev@vger.kernel.org,
idosch@mellanox.com, stable@vger.kernel.org,
gregkh@linuxfoundation.org
In-Reply-To: <20180410.152257.1812747859010556634.davem@davemloft.net>
On Tue, Apr 10, 2018 at 03:22:57PM -0400, David Miller wrote:
>From: Sasha Levin <Alexander.Levin@microsoft.com>
>Date: Tue, 10 Apr 2018 19:08:20 +0000
>
>> The bot tries to take the "dumb" part out of your way, by letting
>> you know from the start which trees this applied/built on and what
>> dependencies it might have. It comes for free, why not use it?
>
>I do this already while I'm processing the -stable queue in patchwork
>and it automatically falls right out of the process I use to extract
>patches out of Linus's GIT tree.
>
>I manually pull the commit out of Linus's tree, verify that it is
>actually the commit I'm interested in, then I do two things:
>
>1) I look at the exact tag that the commit landed in using
> "git describe --contains SHA1_ID"
>
>2) I look at the exact tag that the commit the Fixes: tag
> points at landed using "git describe --contains SHA1_ID"
>
>I double check #2 to see for cases whether the Fixes: tag
>itself was backported to stable trees.
>
>I use these two tag values to organize the -stable queue into
>subdirectories. This guides my patch applying in order to minimize
>useless work.
>
>So I have to do all of this work anyways.
>
>Even if the bot provided these values, I would still double
>check them, every single one of them.
>
>Therefore, the net result from my perspective is that for most
>patches fixing bugs on this list, instead of N list postings,
>there will now be at least N * 2.
Gotcha. I can keep it out from the regular patch flow. How could we
address commits that might have been missed? Let's say I look at commits
that are older than ~1 month, how can I get those looked at again?
>Bots are starting to overwhelm actual content from human beings
>on this list, and I want to put my foot on the brake right now
>before it gets even more out of control.
I think we're just hitting the limitations of using a mailing list. Bots
aren't around to spam everyone for fun, right?
0day bot is spammy because patches fail to compile, syzbot is spammy
because we have tons of bugs users can hit and the stable bot is
spammy because we miss lots of commits that should be in stable.
As the kernel grows, not only the codebase is expanding but also the
tooling around it. While spammy, bots provide valuable input that in the
past would take blood, sweat and tears to acquire. We just need a better
way to consume it rather than blocking off these inputs.
Thanks!
^ permalink raw reply
* Re: [RFC net-next 1/2] net: net-porcfs: Reduce rcu lock critical section
From: Eric Dumazet @ 2018-04-10 20:35 UTC (permalink / raw)
To: David Miller, saeedm; +Cc: netdev
In-Reply-To: <20180410.131649.583776764903333305.davem@davemloft.net>
On 04/10/2018 10:16 AM, David Miller wrote:
>
> The tradeoff here is that now you are doing two unnecessary atomic
> operations per stats dump.
>
> That is what the RCU lock allows us to avoid.
>
dev_hold() and dev_put() are actually per cpu increment and decrements,
pretty cheap these days.
Problem here is that any preemption of the process holding device reference
might trigger warnings in device unregister.
^ permalink raw reply
* Re: [PATCH] net: dsa: Discard frames from unused ports
From: David Miller @ 2018-04-10 19:37 UTC (permalink / raw)
To: andrew; +Cc: netdev
Andrew, I'm working on -stable backports, and hit a snag when pulling
this one into v4.14
4.14 predates the dsa_master_find_slave() helper, but even if I backport
that, the data structures at this point lack the 'type' member for
the test that you added.
Can you put together a v4.14 backport, if applicable, and submit it to
stable@vger.kernel.org CC:'ing me?
Thanks.
^ permalink raw reply
* Re: [patch net] devlink: convert occ_get op to separate registration
From: David Miller @ 2018-04-10 19:22 UTC (permalink / raw)
To: Alexander.Levin; +Cc: jiri, jiri, netdev, idosch, stable, gregkh
In-Reply-To: <20180410190819.GI2341@sasha-vm>
From: Sasha Levin <Alexander.Levin@microsoft.com>
Date: Tue, 10 Apr 2018 19:08:20 +0000
> The bot tries to take the "dumb" part out of your way, by letting
> you know from the start which trees this applied/built on and what
> dependencies it might have. It comes for free, why not use it?
I do this already while I'm processing the -stable queue in patchwork
and it automatically falls right out of the process I use to extract
patches out of Linus's GIT tree.
I manually pull the commit out of Linus's tree, verify that it is
actually the commit I'm interested in, then I do two things:
1) I look at the exact tag that the commit landed in using
"git describe --contains SHA1_ID"
2) I look at the exact tag that the commit the Fixes: tag
points at landed using "git describe --contains SHA1_ID"
I double check #2 to see for cases whether the Fixes: tag
itself was backported to stable trees.
I use these two tag values to organize the -stable queue into
subdirectories. This guides my patch applying in order to minimize
useless work.
So I have to do all of this work anyways.
Even if the bot provided these values, I would still double
check them, every single one of them.
Therefore, the net result from my perspective is that for most
patches fixing bugs on this list, instead of N list postings,
there will now be at least N * 2.
Bots are starting to overwhelm actual content from human beings
on this list, and I want to put my foot on the brake right now
before it gets even more out of control.
Thank you.
^ 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