* Re: [PATCH net v2 2/3] xfrm: Add an activate() offload dev op
From: Steffen Klassert @ 2017-12-01 7:09 UTC (permalink / raw)
To: avivh; +Cc: Herbert Xu, Boris Pismenny, Yossi Kuperman, Yevgeny Kliteynik,
netdev
In-Reply-To: <1511891742-84759-2-git-send-email-avivh@mellanox.com>
On Tue, Nov 28, 2017 at 07:55:41PM +0200, avivh@mellanox.com wrote:
> From: Aviv Heller <avivh@mellanox.com>
>
> Adding the state to the offload device prior to replay init in
> xfrm_state_construct() will result in NULL dereference if a matching
> ESP packet is received in between.
>
> In order to inhibit driver offload logic from processing the state's
> packets prior to the xfrm_state object being completely initialized and
> added to the SADBs, a new activate() operation was added to inform the
> driver the aforementioned conditions have been met.
We discussed this already some time ago, and I still think that
we should fix this by setting XFRM_STATE_VALID only after the
state is fully initialized.
^ permalink raw reply
* Re: [PATCH 2/3] tun: free skb in early errors
From: Jason Wang @ 2017-12-01 7:07 UTC (permalink / raw)
To: wexu, virtualization, netdev, linux-kernel; +Cc: mst, mjrosato
In-Reply-To: <1512107669-27572-3-git-send-email-wexu@redhat.com>
On 2017年12月01日 13:54, wexu@redhat.com wrote:
> From: Wei Xu <wexu@redhat.com>
>
> tun_recvmsg() supports accepting skb by msg_control after
> commit ac77cfd4258f ("tun: support receiving skb through msg_control"),
> the skb if presented should be freed within the function, otherwise it
> would be leaked.
>
> Signed-off-by: Wei Xu <wexu@redhat.com>
> Reported-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
> ---
> drivers/net/tun.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 6a7bde9..5563430 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -2067,14 +2067,17 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
> {
> struct tun_file *tfile = container_of(sock, struct tun_file, socket);
> struct tun_struct *tun = tun_get(tfile);
> + struct sk_buff *skb = m->msg_control;
> int ret;
>
> - if (!tun)
> - return -EBADFD;
> + if (!tun) {
> + ret = -EBADFD;
> + goto out_free_skb;
Unfortunately, you can't to there since tun is NULL.
> + }
>
> if (flags & ~(MSG_DONTWAIT|MSG_TRUNC|MSG_ERRQUEUE)) {
> ret = -EINVAL;
> - goto out;
> + goto out_free_skb;
> }
> if (flags & MSG_ERRQUEUE) {
> ret = sock_recv_errqueue(sock->sk, m, total_len,
> @@ -2087,6 +2090,11 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
> m->msg_flags |= MSG_TRUNC;
> ret = flags & MSG_TRUNC ? ret : total_len;
> }
> + goto out;
We usually don't use goto in the case of success, and you need deal with
the case skb != NULL but iov_iter_count(to) == 0 in tun_do_read().
Thanks
> +
> +out_free_skb:
> + if (skb)
> + kfree_skb(skb);
> out:
> tun_put(tun);
> return ret;
^ permalink raw reply
* Re: [PATCH net v2 3/3] xfrm: Remove redundant state assignment in xfrm_input()
From: Steffen Klassert @ 2017-12-01 7:05 UTC (permalink / raw)
To: avivh; +Cc: Herbert Xu, Boris Pismenny, Yossi Kuperman, Yevgeny Kliteynik,
netdev
In-Reply-To: <1511891742-84759-3-git-send-email-avivh@mellanox.com>
On Tue, Nov 28, 2017 at 07:55:42PM +0200, avivh@mellanox.com wrote:
> From: Aviv Heller <avivh@mellanox.com>
>
> x is already initialized to the same value, above.
>
> Signed-off-by: Aviv Heller <avivh@mellanox.com>
> Signed-off-by: Yevgeny Kliteynik <kliteyn@mellanox.com>
Applied to ipsec-next, thanks!
^ permalink raw reply
* Re: [PATCH net v2 1/3] xfrm: Fix xfrm_input() to verify state is valid when (encap_type < 0)
From: Steffen Klassert @ 2017-12-01 7:04 UTC (permalink / raw)
To: avivh; +Cc: Herbert Xu, Boris Pismenny, Yossi Kuperman, Yevgeny Kliteynik,
netdev
In-Reply-To: <1511891742-84759-1-git-send-email-avivh@mellanox.com>
On Tue, Nov 28, 2017 at 07:55:40PM +0200, avivh@mellanox.com wrote:
> From: Aviv Heller <avivh@mellanox.com>
>
> Code path when (encap_type < 0) does not verify the state is valid
> before progressing.
>
> This will result in a crash if, for instance, x->km.state ==
> XFRM_STATE_ACQ.
>
> Fixes: 7785bba299a8 ("esp: Add a software GRO codepath")
> Signed-off-by: Aviv Heller <avivh@mellanox.com>
> Signed-off-by: Yevgeny Kliteynik <kliteyn@mellanox.com>
Good catch!
Patch applied, thanks Aviv!
^ permalink raw reply
* Re: [PATCH net-next 3/3] xfrm: Add ESN support for IPSec HW offload
From: Steffen Klassert @ 2017-12-01 6:23 UTC (permalink / raw)
To: yossefe
Cc: David S. Miller, Herbert Xu, netdev, linux-kernel, borisp,
kliteyn, yossiku
In-Reply-To: <1511862571-3494-3-git-send-email-yossefe@mellanox.com>
On Tue, Nov 28, 2017 at 11:49:30AM +0200, yossefe@mellanox.com wrote:
> From: Yossef Efraim <yossefe@mellanox.com>
>
> This patch adds ESN support to IPsec device offload.
> Adding new xfrm device operation to synchronize device ESN.
>
> Signed-off-by: Yossef Efraim <yossefe@mellanox.com>
> ---
> include/linux/netdevice.h | 1 +
> include/net/xfrm.h | 12 ++++++++++++
> net/xfrm/xfrm_device.c | 4 ++--
> net/xfrm/xfrm_replay.c | 2 ++
> 4 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 7de7656..d4e9198 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -825,6 +825,7 @@ struct xfrmdev_ops {
> void (*xdo_dev_state_free) (struct xfrm_state *x);
> bool (*xdo_dev_offload_ok) (struct sk_buff *skb,
> struct xfrm_state *x);
> + void (*xdo_dev_state_advance_esn) (struct xfrm_state *x);
We now have a documentation for the xfrm offloadin API in the
ipsec-next tree. Please document the new device operation
there and resubmit.
Thanks!
^ permalink raw reply
* Re: [PATCH net-next 2/3] xfrm: Fix xfrm_dev_state_add to fail for unsupported HW SA option
From: Steffen Klassert @ 2017-12-01 6:21 UTC (permalink / raw)
To: yossefe
Cc: David S. Miller, Herbert Xu, netdev, linux-kernel, borisp,
kliteyn, yossiku
In-Reply-To: <1511862571-3494-2-git-send-email-yossefe@mellanox.com>
On Tue, Nov 28, 2017 at 11:49:29AM +0200, yossefe@mellanox.com wrote:
> From: Yossef Efraim <yossefe@mellanox.com>
>
> xfrm_dev_state_add function returns success for unsupported HW SA options.
> Resulting the calling function to create SW SA without corrlating HW SA.
> Desipte IPSec device offloading option was chosen.
> These not supported HW SA options are hard coded within xfrm_dev_state_add
> function.
> SW backward compatibility will break if we add any of these option as old
> HW will fail with new SW.
>
> This patch changes the behaviour to return -EINVAL in case unsupported
> option is chosen.
> Notifying user application regarding failure and not breaking backward
> compatibility for newly added HW SA options.
>
> Signed-off-by: Yossef Efraim <yossefe@mellanox.com>
Also applied to ipsec-next, thanks a lot!
^ permalink raw reply
* Re: [PATCH net-next 1/3] xfrm: Fix xfrm_replay_overflow_offload_esn
From: Steffen Klassert @ 2017-12-01 6:20 UTC (permalink / raw)
To: yossefe
Cc: David S. Miller, Herbert Xu, netdev, linux-kernel, borisp,
kliteyn, yossiku
In-Reply-To: <1511862571-3494-1-git-send-email-yossefe@mellanox.com>
On Tue, Nov 28, 2017 at 11:49:28AM +0200, yossefe@mellanox.com wrote:
> From: Yossef Efraim <yossefe@mellanox.com>
>
> In case of wrap around, replay_esn->oseq_hi is not updated
> before it is tested for it's actual value, leading function
> to fail with overflow indication and packets being dropped.
>
> This patch updates replay_esn->oseq_hi in the right place.
>
> Fixes: d7dbefc45cf5 ("xfrm: Add xfrm_replay_overflow functions for offloading")
> Signed-off-by: Yossef Efraim <yossefe@mellanox.com>
Applied to ipsec-next, thanks!
^ permalink raw reply
* Re: [PATCH V11 0/5] hash addresses printed with %p
From: Sergey Senozhatsky @ 2017-12-01 6:15 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: David Laight, 'Andrew Morton', Tobin C. Harding,
kernel-hardening@lists.openwall.com, Linus Torvalds,
Jason A. Donenfeld, Theodore Ts'o, Kees Cook, Paolo Bonzini,
Tycho Andersen, Roberts, William C, Tejun Heo, Jordan Glover,
Greg KH, Petr Mladek, Joe Perches, Ian Campbell,
Sergey Senozhatsky, Catalin Marinas <c
In-Reply-To: <20171130102638.GA434@jagdpanzerIV>
On (11/30/17 19:26), Sergey Senozhatsky wrote:
> On (11/30/17 10:23), David Laight wrote:
> [..]
> > > Maybe I'm being thick, but... if we're rendering these addresses
> > > unusable by hashing them, why not just print something like
> > > "<obscured>" in their place? That loses the uniqueness thing but I
> > > wonder how valuable that is in practice?
> >
> > My worry is that is you get a kernel 'oops' print with actual register
> > values you have no easy way of tying an address or address+offset to
> > the corresponding hash(address) printed elsewhere.
>
> print the existing hash:pointer mappings in panic()? [if we can do that]
by this I meant
"when oops_in_progress == 1 then print hash:pointer for %p,
not just hash".
-ss
^ permalink raw reply
* Re: [PATCH ipsec] xfrm: fix XFRMA_OUTPUT_MARK policy entry
From: Steffen Klassert @ 2017-12-01 6:00 UTC (permalink / raw)
To: Michal Kubecek
Cc: Herbert Xu, David S. Miller, Lorenzo Colitti, netdev,
linux-kernel
In-Reply-To: <20171129172356.36CD4A0EE5@unicorn.suse.cz>
On Wed, Nov 29, 2017 at 06:23:56PM +0100, Michal Kubecek wrote:
> This seems to be an obvious typo, NLA_U32 is type of the attribute, not its
> (minimal) length.
>
> Fixes: 077fbac405bf ("net: xfrm: support setting an output mark.")
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
Patch applied, thanks Michal!
^ permalink raw reply
* [PATCH 3/3] tap: free skb if flags error
From: wexu @ 2017-12-01 5:54 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel; +Cc: jasowang, mst, mjrosato, wexu
In-Reply-To: <1512107669-27572-1-git-send-email-wexu@redhat.com>
From: Wei Xu <wexu@redhat.com>
tap_recvmsg() supports accepting skb by msg_control after
commit 3b4ba04acca8 ("tap: support receiving skb from msg_control"),
the skb if presented should be freed within the function, otherwise
it would be leaked.
Signed-off-by: Wei Xu <wexu@redhat.com>
Reported-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
drivers/net/tap.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index e9489b8..1c66b75 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -1154,9 +1154,13 @@ static int tap_recvmsg(struct socket *sock, struct msghdr *m,
size_t total_len, int flags)
{
struct tap_queue *q = container_of(sock, struct tap_queue, sock);
+ struct sk_buff *skb = m->msg_control;
int ret;
- if (flags & ~(MSG_DONTWAIT|MSG_TRUNC))
+ if (flags & ~(MSG_DONTWAIT|MSG_TRUNC)) {
+ if (skb)
+ kfree_skb(skb);
return -EINVAL;
+ }
ret = tap_do_read(q, &m->msg_iter, flags & MSG_DONTWAIT,
m->msg_control);
if (ret > total_len) {
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/3] tun: free skb in early errors
From: wexu @ 2017-12-01 5:54 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel; +Cc: jasowang, mst, mjrosato, wexu
In-Reply-To: <1512107669-27572-1-git-send-email-wexu@redhat.com>
From: Wei Xu <wexu@redhat.com>
tun_recvmsg() supports accepting skb by msg_control after
commit ac77cfd4258f ("tun: support receiving skb through msg_control"),
the skb if presented should be freed within the function, otherwise it
would be leaked.
Signed-off-by: Wei Xu <wexu@redhat.com>
Reported-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
drivers/net/tun.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 6a7bde9..5563430 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2067,14 +2067,17 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
{
struct tun_file *tfile = container_of(sock, struct tun_file, socket);
struct tun_struct *tun = tun_get(tfile);
+ struct sk_buff *skb = m->msg_control;
int ret;
- if (!tun)
- return -EBADFD;
+ if (!tun) {
+ ret = -EBADFD;
+ goto out_free_skb;
+ }
if (flags & ~(MSG_DONTWAIT|MSG_TRUNC|MSG_ERRQUEUE)) {
ret = -EINVAL;
- goto out;
+ goto out_free_skb;
}
if (flags & MSG_ERRQUEUE) {
ret = sock_recv_errqueue(sock->sk, m, total_len,
@@ -2087,6 +2090,11 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
m->msg_flags |= MSG_TRUNC;
ret = flags & MSG_TRUNC ? ret : total_len;
}
+ goto out;
+
+out_free_skb:
+ if (skb)
+ kfree_skb(skb);
out:
tun_put(tun);
return ret;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/3] vhost: fix skb leak in handle_rx()
From: wexu @ 2017-12-01 5:54 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel; +Cc: jasowang, mst, mjrosato, wexu
In-Reply-To: <1512107669-27572-1-git-send-email-wexu@redhat.com>
From: Wei Xu <wexu@redhat.com>
Matthew found a roughly 40% tcp throughput regression with commit
c67df11f(vhost_net: try batch dequing from skb array) as discussed
in the following thread:
https://www.mail-archive.com/netdev@vger.kernel.org/msg187936.html
Eventually we figured out that it was a skb leak in handle_rx()
when sending packets to the VM. This usually happens when a guest
can not drain out vq as fast as vhost fills in, afterwards it sets
off the traffic jam and leaks skb(s) which occurs as no headcount
to send on the vq from vhost side.
This can be avoided by making sure we have got enough headcount
before actually consuming a skb from the batched rx array while
transmitting, which is simply done by moving checking the zero
headcount a bit ahead.
Signed-off-by: Wei Xu <wexu@redhat.com>
Reported-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
drivers/vhost/net.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8d626d7..c7bdeb6 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -778,16 +778,6 @@ static void handle_rx(struct vhost_net *net)
/* On error, stop handling until the next kick. */
if (unlikely(headcount < 0))
goto out;
- if (nvq->rx_array)
- msg.msg_control = vhost_net_buf_consume(&nvq->rxq);
- /* On overrun, truncate and discard */
- if (unlikely(headcount > UIO_MAXIOV)) {
- iov_iter_init(&msg.msg_iter, READ, vq->iov, 1, 1);
- err = sock->ops->recvmsg(sock, &msg,
- 1, MSG_DONTWAIT | MSG_TRUNC);
- pr_debug("Discarded rx packet: len %zd\n", sock_len);
- continue;
- }
/* OK, now we need to know about added descriptors. */
if (!headcount) {
if (unlikely(vhost_enable_notify(&net->dev, vq))) {
@@ -800,6 +790,16 @@ static void handle_rx(struct vhost_net *net)
* they refilled. */
goto out;
}
+ if (nvq->rx_array)
+ msg.msg_control = vhost_net_buf_consume(&nvq->rxq);
+ /* On overrun, truncate and discard */
+ if (unlikely(headcount > UIO_MAXIOV)) {
+ iov_iter_init(&msg.msg_iter, READ, vq->iov, 1, 1);
+ err = sock->ops->recvmsg(sock, &msg,
+ 1, MSG_DONTWAIT | MSG_TRUNC);
+ pr_debug("Discarded rx packet: len %zd\n", sock_len);
+ continue;
+ }
/* We don't need to be notified again. */
iov_iter_init(&msg.msg_iter, READ, vq->iov, in, vhost_len);
fixup = msg.msg_iter;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net,stable v3] vhost: fix a few skb leaks
From: wexu @ 2017-12-01 5:54 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel; +Cc: jasowang, mst, mjrosato, wexu
From: Wei Xu <wexu@redhat.com>
Matthew found a roughly 40% tcp throughput regression with commit
c67df11f(vhost_net: try batch dequing from skb array) as discussed
in the following thread:
https://www.mail-archive.com/netdev@vger.kernel.org/msg187936.html
This is v3.
v3:
- move freeing skb from vhost to tun/tap recvmsg() to not
confuse the callers.
v2:
- add Matthew as the reporter, thanks matthew.
- moving zero headcount check ahead instead of defer consuming skb
due to jason and mst's comment.
- add freeing skb in favor of recvmsg() fails.
Wei Xu (3):
vhost: fix skb leak in handle_rx()
tun: free skb in early errors
tap: free skb if flags error
drivers/net/tap.c | 6 +++++-
drivers/net/tun.c | 14 +++++++++++---
drivers/vhost/net.c | 20 ++++++++++----------
3 files changed, 26 insertions(+), 14 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH ipsec] xfrm: add documentation for xfrm device offload api
From: Steffen Klassert @ 2017-12-01 5:47 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
In-Reply-To: <1511216767-21320-1-git-send-email-shannon.nelson@oracle.com>
On Mon, Nov 20, 2017 at 02:26:07PM -0800, Shannon Nelson wrote:
> Add a writeup on how to use the XFRM device offload API, and
> mention this new file in the index.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
Applied to ipsec-next, thanks a lot for this documentation!
^ permalink raw reply
* Re: [PATCH ipsec-next] net: xfrm: allow clearing socket xfrm policies.
From: Steffen Klassert @ 2017-12-01 5:45 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: netdev, zenczykowski, nharold
In-Reply-To: <20171120102602.144358-1-lorenzo@google.com>
On Mon, Nov 20, 2017 at 07:26:02PM +0900, Lorenzo Colitti wrote:
> Currently it is possible to add or update socket policies, but
> not clear them. Therefore, once a socket policy has been applied,
> the socket cannot be used for unencrypted traffic.
>
> This patch allows (privileged) users to clear socket policies by
> passing in a NULL pointer and zero length argument to the
> {IP,IPV6}_{IPSEC,XFRM}_POLICY setsockopts. This results in both
> the incoming and outgoing policies being cleared.
>
> The simple approach taken in this patch cannot clear socket
> policies in only one direction. If desired this could be added
> in the future, for example by continuing to pass in a length of
> zero (which currently is guaranteed to return EMSGSIZE) and
> making the policy be a pointer to an integer that contains one
> of the XFRM_POLICY_{IN,OUT} enum values.
>
> An alternative would have been to interpret the length as a
> signed integer and use XFRM_POLICY_IN (i.e., 0) to clear the
> input policy and -XFRM_POLICY_OUT (i.e., -1) to clear the output
> policy.
>
> Tested: https://android-review.googlesource.com/539816
> Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
Applied to ipsec-next, thanks Lorenzo!
^ permalink raw reply
* [PATCH net-next 13/13] nfp: bpf: detect load/store sequences lowered from memory copy
From: Jakub Kicinski @ 2017-12-01 5:33 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, Jiong Wang
In-Reply-To: <20171201053300.17503-1-jakub.kicinski@netronome.com>
From: Jiong Wang <jiong.wang@netronome.com>
This patch add the optimization frontend, but adding a new eBPF IR scan
pass "nfp_bpf_opt_ldst_gather".
The pass will traverse the IR to recognize the load/store pairs sequences
that come from lowering of memory copy builtins.
The gathered memory copy information will be kept in the meta info
structure of the first load instruction in the sequence and will be
consumed by the optimization backend added in the previous patches.
NOTE: a sequence with cross memory access doesn't qualify this
optimization, i.e. if one load in the sequence will load from place that
has been written by previous store. This is because when we turn the
sequence into single CPP operation, we are reading all contents at once
into NFP transfer registers, then write them out as a whole. This is not
identical with what the original load/store sequence is doing.
Detecting cross memory access for two random pointers will be difficult,
fortunately under XDP/eBPF's restrictied runtime environment, the copy
normally happen among map, packet data and stack, they do not overlap with
each other.
And for cases supported by NFP, cross memory access will only happen on
PTR_TO_PACKET. Fortunately for this, there is ID information that we could
do accurate memory alias check.
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 237 +++++++++++++++++++++++++++
1 file changed, 237 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index 1b98ef239605..3419ad495962 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -2352,12 +2352,249 @@ static void nfp_bpf_opt_ld_shift(struct nfp_prog *nfp_prog)
}
}
+/* load/store pair that forms memory copy sould look like the following:
+ *
+ * ld_width R, [addr_src + offset_src]
+ * st_width [addr_dest + offset_dest], R
+ *
+ * The destination register of load and source register of store should
+ * be the same, load and store should also perform at the same width.
+ * If either of addr_src or addr_dest is stack pointer, we don't do the
+ * CPP optimization as stack is modelled by registers on NFP.
+ */
+static bool
+curr_pair_is_memcpy(struct nfp_insn_meta *ld_meta,
+ struct nfp_insn_meta *st_meta)
+{
+ struct bpf_insn *ld = &ld_meta->insn;
+ struct bpf_insn *st = &st_meta->insn;
+
+ if (!is_mbpf_load(ld_meta) || !is_mbpf_store(st_meta))
+ return false;
+
+ if (ld_meta->ptr.type != PTR_TO_PACKET)
+ return false;
+
+ if (st_meta->ptr.type != PTR_TO_PACKET)
+ return false;
+
+ if (BPF_SIZE(ld->code) != BPF_SIZE(st->code))
+ return false;
+
+ if (ld->dst_reg != st->src_reg)
+ return false;
+
+ /* There is jump to the store insn in this pair. */
+ if (st_meta->flags & FLAG_INSN_IS_JUMP_DST)
+ return false;
+
+ return true;
+}
+
+/* Currently, we only support chaining load/store pairs if:
+ *
+ * - Their address base registers are the same.
+ * - Their address offsets are in the same order.
+ * - They operate at the same memory width.
+ * - There is no jump into the middle of them.
+ */
+static bool
+curr_pair_chain_with_previous(struct nfp_insn_meta *ld_meta,
+ struct nfp_insn_meta *st_meta,
+ struct bpf_insn *prev_ld,
+ struct bpf_insn *prev_st)
+{
+ u8 prev_size, curr_size, prev_ld_base, prev_st_base, prev_ld_dst;
+ struct bpf_insn *ld = &ld_meta->insn;
+ struct bpf_insn *st = &st_meta->insn;
+ s16 prev_ld_off, prev_st_off;
+
+ /* This pair is the start pair. */
+ if (!prev_ld)
+ return true;
+
+ prev_size = BPF_LDST_BYTES(prev_ld);
+ curr_size = BPF_LDST_BYTES(ld);
+ prev_ld_base = prev_ld->src_reg;
+ prev_st_base = prev_st->dst_reg;
+ prev_ld_dst = prev_ld->dst_reg;
+ prev_ld_off = prev_ld->off;
+ prev_st_off = prev_st->off;
+
+ if (ld->dst_reg != prev_ld_dst)
+ return false;
+
+ if (ld->src_reg != prev_ld_base || st->dst_reg != prev_st_base)
+ return false;
+
+ if (curr_size != prev_size)
+ return false;
+
+ /* There is jump to the head of this pair. */
+ if (ld_meta->flags & FLAG_INSN_IS_JUMP_DST)
+ return false;
+
+ /* Both in ascending order. */
+ if (prev_ld_off + prev_size == ld->off &&
+ prev_st_off + prev_size == st->off)
+ return true;
+
+ /* Both in descending order. */
+ if (ld->off + curr_size == prev_ld_off &&
+ st->off + curr_size == prev_st_off)
+ return true;
+
+ return false;
+}
+
+/* Return TRUE if cross memory access happens. Cross memory access means
+ * store area is overlapping with load area that a later load might load
+ * the value from previous store, for this case we can't treat the sequence
+ * as an memory copy.
+ */
+static bool
+cross_mem_access(struct bpf_insn *ld, struct nfp_insn_meta *head_ld_meta,
+ struct nfp_insn_meta *head_st_meta)
+{
+ s16 head_ld_off, head_st_off, ld_off;
+
+ /* Different pointer types does not overlap. */
+ if (head_ld_meta->ptr.type != head_st_meta->ptr.type)
+ return false;
+
+ /* load and store are both PTR_TO_PACKET, check ID info. */
+ if (head_ld_meta->ptr.id != head_st_meta->ptr.id)
+ return true;
+
+ /* Canonicalize the offsets. Turn all of them against the original
+ * base register.
+ */
+ head_ld_off = head_ld_meta->insn.off + head_ld_meta->ptr.off;
+ head_st_off = head_st_meta->insn.off + head_st_meta->ptr.off;
+ ld_off = ld->off + head_ld_meta->ptr.off;
+
+ /* Ascending order cross. */
+ if (ld_off > head_ld_off &&
+ head_ld_off < head_st_off && ld_off >= head_st_off)
+ return true;
+
+ /* Descending order cross. */
+ if (ld_off < head_ld_off &&
+ head_ld_off > head_st_off && ld_off <= head_st_off)
+ return true;
+
+ return false;
+}
+
+/* This pass try to identify the following instructoin sequences.
+ *
+ * load R, [regA + offA]
+ * store [regB + offB], R
+ * load R, [regA + offA + const_imm_A]
+ * store [regB + offB + const_imm_A], R
+ * load R, [regA + offA + 2 * const_imm_A]
+ * store [regB + offB + 2 * const_imm_A], R
+ * ...
+ *
+ * Above sequence is typically generated by compiler when lowering
+ * memcpy. NFP prefer using CPP instructions to accelerate it.
+ */
+static void nfp_bpf_opt_ldst_gather(struct nfp_prog *nfp_prog)
+{
+ struct nfp_insn_meta *head_ld_meta = NULL;
+ struct nfp_insn_meta *head_st_meta = NULL;
+ struct nfp_insn_meta *meta1, *meta2;
+ struct bpf_insn *prev_ld = NULL;
+ struct bpf_insn *prev_st = NULL;
+ u8 count = 0;
+
+ nfp_for_each_insn_walk2(nfp_prog, meta1, meta2) {
+ struct bpf_insn *ld = &meta1->insn;
+ struct bpf_insn *st = &meta2->insn;
+
+ /* Reset record status if any of the following if true:
+ * - The current insn pair is not load/store.
+ * - The load/store pair doesn't chain with previous one.
+ * - The chained load/store pair crossed with previous pair.
+ * - The chained load/store pair has a total size of memory
+ * copy beyond 128 bytes which is the maximum length a
+ * single NFP CPP command can transfer.
+ */
+ if (!curr_pair_is_memcpy(meta1, meta2) ||
+ !curr_pair_chain_with_previous(meta1, meta2, prev_ld,
+ prev_st) ||
+ (head_ld_meta && (cross_mem_access(ld, head_ld_meta,
+ head_st_meta) ||
+ head_ld_meta->ldst_gather_len >= 128))) {
+ if (!count)
+ continue;
+
+ if (count > 1) {
+ s16 prev_ld_off = prev_ld->off;
+ s16 prev_st_off = prev_st->off;
+ s16 head_ld_off = head_ld_meta->insn.off;
+
+ if (prev_ld_off < head_ld_off) {
+ head_ld_meta->insn.off = prev_ld_off;
+ head_st_meta->insn.off = prev_st_off;
+ head_ld_meta->ldst_gather_len =
+ -head_ld_meta->ldst_gather_len;
+ }
+
+ head_ld_meta->paired_st = &head_st_meta->insn;
+ head_st_meta->skip = true;
+ } else {
+ head_ld_meta->ldst_gather_len = 0;
+ }
+
+ /* If the chain is ended by an load/store pair then this
+ * could serve as the new head of the the next chain.
+ */
+ if (curr_pair_is_memcpy(meta1, meta2)) {
+ head_ld_meta = meta1;
+ head_st_meta = meta2;
+ head_ld_meta->ldst_gather_len =
+ BPF_LDST_BYTES(ld);
+ meta1 = nfp_meta_next(meta1);
+ meta2 = nfp_meta_next(meta2);
+ prev_ld = ld;
+ prev_st = st;
+ count = 1;
+ } else {
+ head_ld_meta = NULL;
+ head_st_meta = NULL;
+ prev_ld = NULL;
+ prev_st = NULL;
+ count = 0;
+ }
+
+ continue;
+ }
+
+ if (!head_ld_meta) {
+ head_ld_meta = meta1;
+ head_st_meta = meta2;
+ } else {
+ meta1->skip = true;
+ meta2->skip = true;
+ }
+
+ head_ld_meta->ldst_gather_len += BPF_LDST_BYTES(ld);
+ meta1 = nfp_meta_next(meta1);
+ meta2 = nfp_meta_next(meta2);
+ prev_ld = ld;
+ prev_st = st;
+ count++;
+ }
+}
+
static int nfp_bpf_optimize(struct nfp_prog *nfp_prog)
{
nfp_bpf_opt_reg_init(nfp_prog);
nfp_bpf_opt_ld_mask(nfp_prog);
nfp_bpf_opt_ld_shift(nfp_prog);
+ nfp_bpf_opt_ldst_gather(nfp_prog);
return 0;
}
--
2.15.0
^ permalink raw reply related
* [PATCH net-next 12/13] nfp: bpf: implement memory bulk copy for length bigger than 32-bytes
From: Jakub Kicinski @ 2017-12-01 5:32 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, Jiong Wang
In-Reply-To: <20171201053300.17503-1-jakub.kicinski@netronome.com>
From: Jiong Wang <jiong.wang@netronome.com>
When the gathered copy length is bigger than 32-bytes and within 128-bytes
(the maximum length a single CPP Pull/Push request can finish), the
strategy of read/write are changeed into:
* Read.
- use direct reference mode when length is within 32-bytes.
- use indirect mode when length is bigger than 32-bytes.
* Write.
- length <= 8-bytes
use write8 (direct_ref).
- length <= 32-byte and 4-bytes aligned
use write32 (direct_ref).
- length <= 32-bytes but not 4-bytes aligned
use write8 (indirect_ref).
- length > 32-bytes and 4-bytes aligned
use write32 (indirect_ref).
- length > 32-bytes and not 4-bytes aligned and <= 40-bytes
use write32 (direct_ref) to finish the first 32-bytes.
use write8 (direct_ref) to finish all remaining hanging part.
- length > 32-bytes and not 4-bytes aligned
use write32 (indirect_ref) to finish those 4-byte aligned parts.
use write8 (direct_ref) to finish all remaining hanging part.
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 52 ++++++++++++++++++++++++----
1 file changed, 45 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index 138568c0eee6..1b98ef239605 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -544,16 +544,18 @@ static int nfp_cpp_memcpy(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
unsigned int i;
u8 xfer_num;
- if (WARN_ON_ONCE(len > 32))
- return -EOPNOTSUPP;
-
off = re_load_imm_any(nfp_prog, meta->insn.off, imm_b(nfp_prog));
src_base = reg_a(meta->insn.src_reg * 2);
xfer_num = round_up(len, 4) / 4;
+ /* Setup PREV_ALU fields to override memory read length. */
+ if (len > 32)
+ wrp_immed(nfp_prog, reg_none(),
+ CMD_OVE_LEN | FIELD_PREP(CMD_OV_LEN, xfer_num - 1));
+
/* Memory read from source addr into transfer-in registers. */
- emit_cmd(nfp_prog, CMD_TGT_READ32_SWAP, CMD_MODE_32b, 0, src_base, off,
- xfer_num - 1, true);
+ emit_cmd_any(nfp_prog, CMD_TGT_READ32_SWAP, CMD_MODE_32b, 0, src_base,
+ off, xfer_num - 1, true, len > 32);
/* Move from transfer-in to transfer-out. */
for (i = 0; i < xfer_num; i++)
@@ -566,18 +568,54 @@ static int nfp_cpp_memcpy(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
emit_cmd(nfp_prog, CMD_TGT_WRITE8_SWAP, CMD_MODE_32b, 0,
reg_a(meta->paired_st->dst_reg * 2), off, len - 1,
true);
- } else if (IS_ALIGNED(len, 4)) {
+ } else if (len <= 32 && IS_ALIGNED(len, 4)) {
/* Use single direct_ref write32. */
emit_cmd(nfp_prog, CMD_TGT_WRITE32_SWAP, CMD_MODE_32b, 0,
reg_a(meta->paired_st->dst_reg * 2), off, xfer_num - 1,
true);
- } else {
+ } else if (len <= 32) {
/* Use single indirect_ref write8. */
wrp_immed(nfp_prog, reg_none(),
CMD_OVE_LEN | FIELD_PREP(CMD_OV_LEN, len - 1));
emit_cmd_indir(nfp_prog, CMD_TGT_WRITE8_SWAP, CMD_MODE_32b, 0,
reg_a(meta->paired_st->dst_reg * 2), off,
len - 1, true);
+ } else if (IS_ALIGNED(len, 4)) {
+ /* Use single indirect_ref write32. */
+ wrp_immed(nfp_prog, reg_none(),
+ CMD_OVE_LEN | FIELD_PREP(CMD_OV_LEN, xfer_num - 1));
+ emit_cmd_indir(nfp_prog, CMD_TGT_WRITE32_SWAP, CMD_MODE_32b, 0,
+ reg_a(meta->paired_st->dst_reg * 2), off,
+ xfer_num - 1, true);
+ } else if (len <= 40) {
+ /* Use one direct_ref write32 to write the first 32-bytes, then
+ * another direct_ref write8 to write the remaining bytes.
+ */
+ emit_cmd(nfp_prog, CMD_TGT_WRITE32_SWAP, CMD_MODE_32b, 0,
+ reg_a(meta->paired_st->dst_reg * 2), off, 7,
+ true);
+
+ off = re_load_imm_any(nfp_prog, meta->paired_st->off + 32,
+ imm_b(nfp_prog));
+ emit_cmd(nfp_prog, CMD_TGT_WRITE8_SWAP, CMD_MODE_32b, 8,
+ reg_a(meta->paired_st->dst_reg * 2), off, len - 33,
+ true);
+ } else {
+ /* Use one indirect_ref write32 to write 4-bytes aligned length,
+ * then another direct_ref write8 to write the remaining bytes.
+ */
+ u8 new_off;
+
+ wrp_immed(nfp_prog, reg_none(),
+ CMD_OVE_LEN | FIELD_PREP(CMD_OV_LEN, xfer_num - 2));
+ emit_cmd_indir(nfp_prog, CMD_TGT_WRITE32_SWAP, CMD_MODE_32b, 0,
+ reg_a(meta->paired_st->dst_reg * 2), off,
+ xfer_num - 2, true);
+ new_off = meta->paired_st->off + (xfer_num - 1) * 4;
+ off = re_load_imm_any(nfp_prog, new_off, imm_b(nfp_prog));
+ emit_cmd(nfp_prog, CMD_TGT_WRITE8_SWAP, CMD_MODE_32b,
+ xfer_num - 1, reg_a(meta->paired_st->dst_reg * 2), off,
+ (len & 0x3) - 1, true);
}
/* TODO: The following extra load is to make sure data flow be identical
--
2.15.0
^ permalink raw reply related
* [PATCH net-next 11/13] nfp: bpf: implement memory bulk copy for length within 32-bytes
From: Jakub Kicinski @ 2017-12-01 5:32 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, Jiong Wang
In-Reply-To: <20171201053300.17503-1-jakub.kicinski@netronome.com>
From: Jiong Wang <jiong.wang@netronome.com>
For NFP, we want to re-group a sequence of load/store pairs lowered from
memcpy/memmove into single memory bulk operation which then could be
accelerated using NFP CPP bus.
This patch extends the existing load/store auxiliary information by adding
two new fields:
struct bpf_insn *paired_st;
s16 ldst_gather_len;
Both fields are supposed to be carried by the the load instruction at the
head of the sequence. "paired_st" is the corresponding store instruction at
the head and "ldst_gather_len" is the gathered length.
If "ldst_gather_len" is negative, then the sequence is doing memory
load/store in descending order, otherwise it is in ascending order. We need
this information to detect overlapped memory access.
This patch then optimize memory bulk copy when the copy length is within
32-bytes.
The strategy of read/write used is:
* Read.
Use read32 (direct_ref), always.
* Write.
- length <= 8-bytes
write8 (direct_ref).
- length <= 32-bytes and is 4-byte aligned
write32 (direct_ref).
- length <= 32-bytes but is not 4-byte aligned
write8 (indirect_ref).
NOTE: the optimization should not change program semantics. The destination
register of the last load instruction should contain the same value before
and after this optimization.
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 113 ++++++++++++++++++++++++++
drivers/net/ethernet/netronome/nfp/bpf/main.h | 4 +
drivers/net/ethernet/netronome/nfp/nfp_asm.c | 1 +
drivers/net/ethernet/netronome/nfp/nfp_asm.h | 4 +
4 files changed, 122 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index da4e106d3b16..138568c0eee6 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -154,6 +154,13 @@ emit_cmd(struct nfp_prog *nfp_prog, enum cmd_tgt_map op, u8 mode, u8 xfer,
emit_cmd_any(nfp_prog, op, mode, xfer, lreg, rreg, size, sync, false);
}
+static void
+emit_cmd_indir(struct nfp_prog *nfp_prog, enum cmd_tgt_map op, u8 mode, u8 xfer,
+ swreg lreg, swreg rreg, u8 size, bool sync)
+{
+ emit_cmd_any(nfp_prog, op, mode, xfer, lreg, rreg, size, sync, true);
+}
+
static void
__emit_br(struct nfp_prog *nfp_prog, enum br_mask mask, enum br_ev_pip ev_pip,
enum br_ctx_signal_state css, u16 addr, u8 defer)
@@ -515,6 +522,109 @@ static void wrp_reg_mov(struct nfp_prog *nfp_prog, u16 dst, u16 src)
wrp_mov(nfp_prog, reg_both(dst), reg_b(src));
}
+/* wrp_reg_subpart() - load @field_len bytes from @offset of @src, write the
+ * result to @dst from low end.
+ */
+static void
+wrp_reg_subpart(struct nfp_prog *nfp_prog, swreg dst, swreg src, u8 field_len,
+ u8 offset)
+{
+ enum shf_sc sc = offset ? SHF_SC_R_SHF : SHF_SC_NONE;
+ u8 mask = (1 << field_len) - 1;
+
+ emit_ld_field_any(nfp_prog, dst, mask, src, sc, offset * 8, true);
+}
+
+/* NFP has Command Push Pull bus which supports bluk memory operations. */
+static int nfp_cpp_memcpy(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
+{
+ bool descending_seq = meta->ldst_gather_len < 0;
+ s16 len = abs(meta->ldst_gather_len);
+ swreg src_base, off;
+ unsigned int i;
+ u8 xfer_num;
+
+ if (WARN_ON_ONCE(len > 32))
+ return -EOPNOTSUPP;
+
+ off = re_load_imm_any(nfp_prog, meta->insn.off, imm_b(nfp_prog));
+ src_base = reg_a(meta->insn.src_reg * 2);
+ xfer_num = round_up(len, 4) / 4;
+
+ /* Memory read from source addr into transfer-in registers. */
+ emit_cmd(nfp_prog, CMD_TGT_READ32_SWAP, CMD_MODE_32b, 0, src_base, off,
+ xfer_num - 1, true);
+
+ /* Move from transfer-in to transfer-out. */
+ for (i = 0; i < xfer_num; i++)
+ wrp_mov(nfp_prog, reg_xfer(i), reg_xfer(i));
+
+ off = re_load_imm_any(nfp_prog, meta->paired_st->off, imm_b(nfp_prog));
+
+ if (len <= 8) {
+ /* Use single direct_ref write8. */
+ emit_cmd(nfp_prog, CMD_TGT_WRITE8_SWAP, CMD_MODE_32b, 0,
+ reg_a(meta->paired_st->dst_reg * 2), off, len - 1,
+ true);
+ } else if (IS_ALIGNED(len, 4)) {
+ /* Use single direct_ref write32. */
+ emit_cmd(nfp_prog, CMD_TGT_WRITE32_SWAP, CMD_MODE_32b, 0,
+ reg_a(meta->paired_st->dst_reg * 2), off, xfer_num - 1,
+ true);
+ } else {
+ /* Use single indirect_ref write8. */
+ wrp_immed(nfp_prog, reg_none(),
+ CMD_OVE_LEN | FIELD_PREP(CMD_OV_LEN, len - 1));
+ emit_cmd_indir(nfp_prog, CMD_TGT_WRITE8_SWAP, CMD_MODE_32b, 0,
+ reg_a(meta->paired_st->dst_reg * 2), off,
+ len - 1, true);
+ }
+
+ /* TODO: The following extra load is to make sure data flow be identical
+ * before and after we do memory copy optimization.
+ *
+ * The load destination register is not guaranteed to be dead, so we
+ * need to make sure it is loaded with the value the same as before
+ * this transformation.
+ *
+ * These extra loads could be removed once we have accurate register
+ * usage information.
+ */
+ if (descending_seq)
+ xfer_num = 0;
+ else if (BPF_SIZE(meta->insn.code) != BPF_DW)
+ xfer_num = xfer_num - 1;
+ else
+ xfer_num = xfer_num - 2;
+
+ switch (BPF_SIZE(meta->insn.code)) {
+ case BPF_B:
+ wrp_reg_subpart(nfp_prog, reg_both(meta->insn.dst_reg * 2),
+ reg_xfer(xfer_num), 1,
+ IS_ALIGNED(len, 4) ? 3 : (len & 3) - 1);
+ break;
+ case BPF_H:
+ wrp_reg_subpart(nfp_prog, reg_both(meta->insn.dst_reg * 2),
+ reg_xfer(xfer_num), 2, (len & 3) ^ 2);
+ break;
+ case BPF_W:
+ wrp_mov(nfp_prog, reg_both(meta->insn.dst_reg * 2),
+ reg_xfer(0));
+ break;
+ case BPF_DW:
+ wrp_mov(nfp_prog, reg_both(meta->insn.dst_reg * 2),
+ reg_xfer(xfer_num));
+ wrp_mov(nfp_prog, reg_both(meta->insn.dst_reg * 2 + 1),
+ reg_xfer(xfer_num + 1));
+ break;
+ }
+
+ if (BPF_SIZE(meta->insn.code) != BPF_DW)
+ wrp_immed(nfp_prog, reg_both(meta->insn.dst_reg * 2 + 1), 0);
+
+ return 0;
+}
+
static int
data_ld(struct nfp_prog *nfp_prog, swreg offset, u8 dst_gpr, int size)
{
@@ -1490,6 +1600,9 @@ static int
mem_ldx(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
unsigned int size)
{
+ if (meta->ldst_gather_len)
+ return nfp_cpp_memcpy(nfp_prog, meta);
+
if (meta->ptr.type == PTR_TO_CTX) {
if (nfp_prog->type == BPF_PROG_TYPE_XDP)
return mem_ldx_xdp(nfp_prog, meta, size);
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index 20ef0adb2931..5884291ddba5 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -95,6 +95,8 @@ typedef int (*instr_cb_t)(struct nfp_prog *, struct nfp_insn_meta *);
* struct nfp_insn_meta - BPF instruction wrapper
* @insn: BPF instruction
* @ptr: pointer type for memory operations
+ * @ldst_gather_len: memcpy length gathered from load/store sequence
+ * @paired_st: the paired store insn at the head of the sequence
* @ptr_not_const: pointer is not always constant
* @jmp_dst: destination info for jump instructions
* @off: index of first generated machine instruction (in nfp_prog.prog)
@@ -109,6 +111,8 @@ struct nfp_insn_meta {
union {
struct {
struct bpf_reg_state ptr;
+ struct bpf_insn *paired_st;
+ s16 ldst_gather_len;
bool ptr_not_const;
};
struct nfp_insn_meta *jmp_dst;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_asm.c b/drivers/net/ethernet/netronome/nfp/nfp_asm.c
index da277386077c..d3610987fb07 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_asm.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_asm.c
@@ -41,6 +41,7 @@
const struct cmd_tgt_act cmd_tgt_act[__CMD_TGT_MAP_SIZE] = {
[CMD_TGT_WRITE8_SWAP] = { 0x02, 0x42 },
+ [CMD_TGT_WRITE32_SWAP] = { 0x02, 0x5f },
[CMD_TGT_READ8] = { 0x01, 0x43 },
[CMD_TGT_READ32] = { 0x00, 0x5c },
[CMD_TGT_READ32_LE] = { 0x01, 0x5c },
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_asm.h b/drivers/net/ethernet/netronome/nfp/nfp_asm.h
index 6ff842a15e5d..98803f9f40b6 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_asm.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_asm.h
@@ -220,6 +220,7 @@ struct cmd_tgt_act {
enum cmd_tgt_map {
CMD_TGT_READ8,
CMD_TGT_WRITE8_SWAP,
+ CMD_TGT_WRITE32_SWAP,
CMD_TGT_READ32,
CMD_TGT_READ32_LE,
CMD_TGT_READ32_SWAP,
@@ -241,6 +242,9 @@ enum cmd_ctx_swap {
CMD_CTX_NO_SWAP = 3,
};
+#define CMD_OVE_LEN BIT(7)
+#define CMD_OV_LEN GENMASK(12, 8)
+
#define OP_LCSR_BASE 0x0fc00000000ULL
#define OP_LCSR_A_SRC 0x000000003ffULL
#define OP_LCSR_B_SRC 0x000000ffc00ULL
--
2.15.0
^ permalink raw reply related
* [PATCH net-next 09/13] nfp: bpf: encode indirect commands
From: Jakub Kicinski @ 2017-12-01 5:32 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, Jakub Kicinski, Jiong Wang
In-Reply-To: <20171201053300.17503-1-jakub.kicinski@netronome.com>
Add support for emitting commands with field overwrites.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 17 +++++++++++++----
drivers/net/ethernet/netronome/nfp/nfp_asm.h | 3 ++-
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index 024b44089623..da4e106d3b16 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -96,7 +96,7 @@ nfp_prog_offset_to_index(struct nfp_prog *nfp_prog, unsigned int offset)
/* --- Emitters --- */
static void
__emit_cmd(struct nfp_prog *nfp_prog, enum cmd_tgt_map op,
- u8 mode, u8 xfer, u8 areg, u8 breg, u8 size, bool sync)
+ u8 mode, u8 xfer, u8 areg, u8 breg, u8 size, bool sync, bool indir)
{
enum cmd_ctx_swap ctx;
u64 insn;
@@ -114,14 +114,15 @@ __emit_cmd(struct nfp_prog *nfp_prog, enum cmd_tgt_map op,
FIELD_PREP(OP_CMD_CNT, size) |
FIELD_PREP(OP_CMD_SIG, sync) |
FIELD_PREP(OP_CMD_TGT_CMD, cmd_tgt_act[op].tgt_cmd) |
+ FIELD_PREP(OP_CMD_INDIR, indir) |
FIELD_PREP(OP_CMD_MODE, mode);
nfp_prog_push(nfp_prog, insn);
}
static void
-emit_cmd(struct nfp_prog *nfp_prog, enum cmd_tgt_map op,
- u8 mode, u8 xfer, swreg lreg, swreg rreg, u8 size, bool sync)
+emit_cmd_any(struct nfp_prog *nfp_prog, enum cmd_tgt_map op, u8 mode, u8 xfer,
+ swreg lreg, swreg rreg, u8 size, bool sync, bool indir)
{
struct nfp_insn_re_regs reg;
int err;
@@ -142,7 +143,15 @@ emit_cmd(struct nfp_prog *nfp_prog, enum cmd_tgt_map op,
return;
}
- __emit_cmd(nfp_prog, op, mode, xfer, reg.areg, reg.breg, size, sync);
+ __emit_cmd(nfp_prog, op, mode, xfer, reg.areg, reg.breg, size, sync,
+ indir);
+}
+
+static void
+emit_cmd(struct nfp_prog *nfp_prog, enum cmd_tgt_map op, u8 mode, u8 xfer,
+ swreg lreg, swreg rreg, u8 size, bool sync)
+{
+ emit_cmd_any(nfp_prog, op, mode, xfer, lreg, rreg, size, sync, false);
}
static void
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_asm.h b/drivers/net/ethernet/netronome/nfp/nfp_asm.h
index 74d0c11ab2f9..6ff842a15e5d 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_asm.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_asm.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016 Netronome Systems, Inc.
+ * Copyright (C) 2016-2017 Netronome Systems, Inc.
*
* This software is dual licensed under the GNU General License Version 2,
* June 1991 as shown in the file COPYING in the top-level directory of this
@@ -209,6 +209,7 @@ enum alu_dst_ab {
#define OP_CMD_CNT 0x0000e000000ULL
#define OP_CMD_SIG 0x000f0000000ULL
#define OP_CMD_TGT_CMD 0x07f00000000ULL
+#define OP_CMD_INDIR 0x20000000000ULL
#define OP_CMD_MODE 0x1c0000000000ULL
struct cmd_tgt_act {
--
2.15.0
^ permalink raw reply related
* [PATCH net-next 10/13] nfp: bpf: factor out is_mbpf_load & is_mbpf_store
From: Jakub Kicinski @ 2017-12-01 5:32 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, Jiong Wang
In-Reply-To: <20171201053300.17503-1-jakub.kicinski@netronome.com>
From: Jiong Wang <jiong.wang@netronome.com>
It is usual that we need to check if one BPF insn is for loading/storeing
data from/to memory.
Therefore, it makes sense to factor out related code to become common
helper functions.
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/main.h | 10 ++++++++++
drivers/net/ethernet/netronome/nfp/bpf/verifier.c | 4 ++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index 99da1d34dd0e..20ef0adb2931 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -144,6 +144,16 @@ static inline u8 mbpf_mode(const struct nfp_insn_meta *meta)
return BPF_MODE(meta->insn.code);
}
+static inline bool is_mbpf_load(const struct nfp_insn_meta *meta)
+{
+ return (meta->insn.code & ~BPF_SIZE_MASK) == (BPF_LDX | BPF_MEM);
+}
+
+static inline bool is_mbpf_store(const struct nfp_insn_meta *meta)
+{
+ return (meta->insn.code & ~BPF_SIZE_MASK) == (BPF_STX | BPF_MEM);
+}
+
/**
* struct nfp_prog - nfp BPF program
* @prog: machine code
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
index cca67730b91f..d2bf29c90226 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
@@ -180,10 +180,10 @@ nfp_verify_insn(struct bpf_verifier_env *env, int insn_idx, int prev_insn_idx)
if (meta->insn.code == (BPF_JMP | BPF_EXIT))
return nfp_bpf_check_exit(nfp_prog, env);
- if ((meta->insn.code & ~BPF_SIZE_MASK) == (BPF_LDX | BPF_MEM))
+ if (is_mbpf_load(meta))
return nfp_bpf_check_ptr(nfp_prog, meta, env,
meta->insn.src_reg);
- if ((meta->insn.code & ~BPF_SIZE_MASK) == (BPF_STX | BPF_MEM))
+ if (is_mbpf_store(meta))
return nfp_bpf_check_ptr(nfp_prog, meta, env,
meta->insn.dst_reg);
--
2.15.0
^ permalink raw reply related
* [PATCH net-next 08/13] nfp: bpf: correct the encoding for No-Dest immed
From: Jakub Kicinski @ 2017-12-01 5:32 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, Jiong Wang
In-Reply-To: <20171201053300.17503-1-jakub.kicinski@netronome.com>
From: Jiong Wang <jiong.wang@netronome.com>
When immed is used with No-Dest, the emitter should use reg.dst instead of
reg.areg for the destination, using the latter will actually encode
register zero.
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index 54915a3b8a7e..024b44089623 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -224,9 +224,11 @@ emit_immed(struct nfp_prog *nfp_prog, swreg dst, u16 imm,
return;
}
- __emit_immed(nfp_prog, reg.areg, reg.breg, imm >> 8, width,
- invert, shift, reg.wr_both,
- reg.dst_lmextn, reg.src_lmextn);
+ /* Use reg.dst when destination is No-Dest. */
+ __emit_immed(nfp_prog,
+ swreg_type(dst) == NN_REG_NONE ? reg.dst : reg.areg,
+ reg.breg, imm >> 8, width, invert, shift,
+ reg.wr_both, reg.dst_lmextn, reg.src_lmextn);
}
static void
--
2.15.0
^ permalink raw reply related
* [PATCH net-next 07/13] nfp: bpf: relax source operands check
From: Jakub Kicinski @ 2017-12-01 5:32 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, Jiong Wang
In-Reply-To: <20171201053300.17503-1-jakub.kicinski@netronome.com>
From: Jiong Wang <jiong.wang@netronome.com>
The NFP normally requires the source operands to be difference addressing
modes, but we should rule out the very special NN_REG_NONE type.
There are instruction that ignores both A/B operands, for example:
local_csr_rd
For these instructions, we might pass the same operand type, NN_REG_NONE,
for both A/B operands.
NOTE: in current NFP ISA, it is only possible for instructions with
unrestricted operands to take none operands, but in case there is new and
similar instructoin in restricted form, they would follow similar rules,
so swreg_to_restricted is updated as well.
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/nfp_asm.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_asm.c b/drivers/net/ethernet/netronome/nfp/nfp_asm.c
index 830f6de25f47..da277386077c 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_asm.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_asm.c
@@ -120,7 +120,8 @@ int swreg_to_unrestricted(swreg dst, swreg lreg, swreg rreg,
reg->dst = nfp_swreg_to_unreg(dst, true);
/* Decode source operands */
- if (swreg_type(lreg) == swreg_type(rreg))
+ if (swreg_type(lreg) == swreg_type(rreg) &&
+ swreg_type(lreg) != NN_REG_NONE)
return -EFAULT;
if (swreg_type(lreg) == NN_REG_GPR_B ||
@@ -200,7 +201,8 @@ int swreg_to_restricted(swreg dst, swreg lreg, swreg rreg,
reg->dst = nfp_swreg_to_rereg(dst, true, false, NULL);
/* Decode source operands */
- if (swreg_type(lreg) == swreg_type(rreg))
+ if (swreg_type(lreg) == swreg_type(rreg) &&
+ swreg_type(lreg) != NN_REG_NONE)
return -EFAULT;
if (swreg_type(lreg) == NN_REG_GPR_B ||
--
2.15.0
^ permalink raw reply related
* [PATCH net-next 06/13] nfp: bpf: don't do ld/shifts combination if shifts are jump destination
From: Jakub Kicinski @ 2017-12-01 5:32 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, Jiong Wang
In-Reply-To: <20171201053300.17503-1-jakub.kicinski@netronome.com>
From: Jiong Wang <jiong.wang@netronome.com>
If any of the shift insns in the ld/shift sequence is jump destination,
don't do combination.
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index f2317b764222..54915a3b8a7e 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -2181,6 +2181,10 @@ static void nfp_bpf_opt_ld_shift(struct nfp_prog *nfp_prog)
if (next1.imm != 0x20 || next2.imm != 0x20)
continue;
+ if (meta2->flags & FLAG_INSN_IS_JUMP_DST ||
+ meta3->flags & FLAG_INSN_IS_JUMP_DST)
+ continue;
+
meta2->skip = true;
meta3->skip = true;
}
--
2.15.0
^ permalink raw reply related
* [PATCH net-next 05/13] nfp: bpf: don't do ld/mask combination if mask is jump destination
From: Jakub Kicinski @ 2017-12-01 5:32 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, Jiong Wang
In-Reply-To: <20171201053300.17503-1-jakub.kicinski@netronome.com>
From: Jiong Wang <jiong.wang@netronome.com>
If the mask insn in the ld/mask pair is jump destination, then don't do
combination.
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index f76659ecb654..f2317b764222 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -2142,6 +2142,9 @@ static void nfp_bpf_opt_ld_mask(struct nfp_prog *nfp_prog)
if (next.src_reg || next.dst_reg)
continue;
+ if (meta2->flags & FLAG_INSN_IS_JUMP_DST)
+ continue;
+
meta2->skip = true;
}
}
--
2.15.0
^ permalink raw reply related
* [PATCH net-next 04/13] nfp: bpf: flag jump destination to guide insn combine optimizations
From: Jakub Kicinski @ 2017-12-01 5:32 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, Jiong Wang
In-Reply-To: <20171201053300.17503-1-jakub.kicinski@netronome.com>
From: Jiong Wang <jiong.wang@netronome.com>
NFP eBPF offload JIT engine is doing some instruction combine based
optimizations which however must not be safe if the combined sequences
are across basic block boarders.
Currently, there are post checks during fixing jump destinations. If the
jump destination is found to be eBPF insn that has been combined into
another one, then JIT engine will raise error and abort.
This is not optimal. The JIT engine ought to disable the optimization on
such cross-bb-border sequences instead of abort.
As there is no control flow information in eBPF infrastructure that we
can't do basic block based optimizations, this patch extends the existing
jump destination record pass to also flag the jump destination, then in
instruction combine passes we could skip the optimizations if insns in the
sequence are jump targets.
Suggested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/main.h | 4 ++++
drivers/net/ethernet/netronome/nfp/bpf/offload.c | 1 +
2 files changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index e488656f406c..99da1d34dd0e 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -89,6 +89,8 @@ typedef int (*instr_cb_t)(struct nfp_prog *, struct nfp_insn_meta *);
#define nfp_meta_next(meta) list_next_entry(meta, l)
#define nfp_meta_prev(meta) list_prev_entry(meta, l)
+#define FLAG_INSN_IS_JUMP_DST BIT(0)
+
/**
* struct nfp_insn_meta - BPF instruction wrapper
* @insn: BPF instruction
@@ -97,6 +99,7 @@ typedef int (*instr_cb_t)(struct nfp_prog *, struct nfp_insn_meta *);
* @jmp_dst: destination info for jump instructions
* @off: index of first generated machine instruction (in nfp_prog.prog)
* @n: eBPF instruction number
+ * @flags: eBPF instruction extra optimization flags
* @skip: skip this instruction (optimized out)
* @double_cb: callback for second part of the instruction
* @l: link on nfp_prog->insns list
@@ -112,6 +115,7 @@ struct nfp_insn_meta {
};
unsigned int off;
unsigned short n;
+ unsigned short flags;
bool skip;
instr_cb_t double_cb;
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index 240db663d83f..377976ce92dd 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -83,6 +83,7 @@ nfp_prog_prepare(struct nfp_prog *nfp_prog, const struct bpf_insn *prog,
cnt);
meta->jmp_dst = dst_meta;
+ dst_meta->flags |= FLAG_INSN_IS_JUMP_DST;
}
}
--
2.15.0
^ 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