* [PATCH bpf-next v3 13/13] tools: bpftool: allow reuse of maps with bpftool prog load
From: Jakub Kicinski @ 2018-07-10 21:43 UTC (permalink / raw)
To: alexei.starovoitov, daniel, Andrey Ignatov
Cc: oss-drivers, netdev, Jakub Kicinski
In-Reply-To: <20180710214307.4834-1-jakub.kicinski@netronome.com>
Add map parameter to prog load which will allow reuse of existing
maps instead of creating new ones.
We need feature detection and compat code for reallocarray, since
it's not available in many libc versions.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
.../bpftool/Documentation/bpftool-prog.rst | 20 ++-
tools/bpf/bpftool/bash-completion/bpftool | 67 +++++++-
tools/bpf/bpftool/main.h | 3 +
tools/bpf/bpftool/map.c | 4 +-
tools/bpf/bpftool/prog.c | 148 ++++++++++++++++--
5 files changed, 219 insertions(+), 23 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index e53e1ad2caf0..64156a16d530 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -24,9 +24,10 @@ MAP COMMANDS
| **bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes** | **visual**}]
| **bpftool** **prog dump jited** *PROG* [{**file** *FILE* | **opcodes**}]
| **bpftool** **prog pin** *PROG* *FILE*
-| **bpftool** **prog load** *OBJ* *FILE* [**type** *TYPE*] [**dev** *NAME*]
+| **bpftool** **prog load** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
| **bpftool** **prog help**
|
+| *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
| *PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
| *TYPE* := {
| **socket** | **kprobe** | **kretprobe** | **classifier** | **action** |
@@ -73,10 +74,17 @@ DESCRIPTION
Note: *FILE* must be located in *bpffs* mount.
- **bpftool prog load** *OBJ* *FILE* [**type** *TYPE*] [**dev** *NAME*]
+ **bpftool prog load** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
Load bpf program from binary *OBJ* and pin as *FILE*.
**type** is optional, if not specified program type will be
inferred from section names.
+ By default bpftool will create new maps as declared in the ELF
+ object being loaded. **map** parameter allows for the reuse
+ of existing maps. It can be specified multiple times, each
+ time for a different map. *IDX* refers to index of the map
+ to be replaced in the ELF file counting from 0, while *NAME*
+ allows to replace a map by name. *MAP* specifies the map to
+ use, referring to it by **id** or through a **pinned** file.
If **dev** *NAME* is specified program will be loaded onto
given networking device (offload).
@@ -172,6 +180,14 @@ EXAMPLES
mov %rbx,0x0(%rbp)
48 89 5d 00
+|
+| **# bpftool prog load xdp1_kern.o /sys/fs/bpf/xdp1 type xdp map name rxcnt id 7**
+| **# bpftool prog show pinned /sys/fs/bpf/xdp1**
+| 9: xdp name xdp_prog1 tag 539ec6ce11b52f98 gpl
+| loaded_at 2018-06-25T16:17:31-0700 uid 0
+| xlated 488B jited 336B memlock 4096B map_ids 7
+| **# rm /sys/fs/bpf/xdp1**
+|
SEE ALSO
========
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index caf8711993be..598066c40191 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -99,6 +99,29 @@ _bpftool_get_prog_tags()
command sed -n 's/.*"tag": "\(.*\)",$/\1/p' )" -- "$cur" ) )
}
+_bpftool_get_obj_map_names()
+{
+ local obj
+
+ obj=$1
+
+ maps=$(objdump -j maps -t $obj 2>/dev/null | \
+ command awk '/g . maps/ {print $NF}')
+
+ COMPREPLY+=( $( compgen -W "$maps" -- "$cur" ) )
+}
+
+_bpftool_get_obj_map_idxs()
+{
+ local obj
+
+ obj=$1
+
+ nmaps=$(objdump -j maps -t $obj 2>/dev/null | grep -c 'g . maps')
+
+ COMPREPLY+=( $( compgen -W "$(seq 0 $((nmaps - 1)))" -- "$cur" ) )
+}
+
_sysfs_get_netdevs()
{
COMPREPLY+=( $( compgen -W "$( ls /sys/class/net 2>/dev/null )" -- \
@@ -220,12 +243,14 @@ _bpftool()
# Completion depends on object and command in use
case $object in
prog)
- case $prev in
- id)
- _bpftool_get_prog_ids
- return 0
- ;;
- esac
+ if [[ $command != "load" ]]; then
+ case $prev in
+ id)
+ _bpftool_get_prog_ids
+ return 0
+ ;;
+ esac
+ fi
local PROG_TYPE='id pinned tag'
case $command in
@@ -268,22 +293,52 @@ _bpftool()
return 0
;;
load)
+ local obj
+
if [[ ${#words[@]} -lt 6 ]]; then
_filedir
return 0
fi
+ obj=${words[3]}
+
+ if [[ ${words[-4]} == "map" ]]; then
+ COMPREPLY=( $( compgen -W "id pinned" -- "$cur" ) )
+ return 0
+ fi
+ if [[ ${words[-3]} == "map" ]]; then
+ if [[ ${words[-2]} == "idx" ]]; then
+ _bpftool_get_obj_map_idxs $obj
+ elif [[ ${words[-2]} == "name" ]]; then
+ _bpftool_get_obj_map_names $obj
+ fi
+ return 0
+ fi
+ if [[ ${words[-2]} == "map" ]]; then
+ COMPREPLY=( $( compgen -W "idx name" -- "$cur" ) )
+ return 0
+ fi
+
case $prev in
type)
COMPREPLY=( $( compgen -W "socket kprobe kretprobe classifier action tracepoint raw_tracepoint xdp perf_event cgroup/skb cgroup/sock cgroup/dev lwt_in lwt_out lwt_xmit lwt_seg6local sockops sk_skb sk_msg lirc_mode2 cgroup/bind4 cgroup/bind6 cgroup/connect4 cgroup/connect6 cgroup/sendmsg4 cgroup/sendmsg6 cgroup/post_bind4 cgroup/post_bind6" -- \
"$cur" ) )
return 0
;;
+ id)
+ _bpftool_get_map_ids
+ return 0
+ ;;
+ pinned)
+ _filedir
+ return 0
+ ;;
dev)
_sysfs_get_netdevs
return 0
;;
*)
+ COMPREPLY=( $( compgen -W "map" -- "$cur" ) )
_bpftool_once_attr 'type'
_bpftool_once_attr 'dev'
return 0
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 1e02e4031693..41004bb2644a 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -75,6 +75,8 @@
"PROG := { id PROG_ID | pinned FILE | tag PROG_TAG }"
#define HELP_SPEC_OPTIONS \
"OPTIONS := { {-j|--json} [{-p|--pretty}] | {-f|--bpffs} }"
+#define HELP_SPEC_MAP \
+ "MAP := { id MAP_ID | pinned FILE }"
enum bpf_obj_type {
BPF_OBJ_UNKNOWN,
@@ -136,6 +138,7 @@ int do_cgroup(int argc, char **arg);
int do_perf(int argc, char **arg);
int prog_parse_fd(int *argc, char ***argv);
+int map_parse_fd(int *argc, char ***argv);
int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len);
void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 5989e1575ae4..e2baec1122fb 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -93,7 +93,7 @@ static void *alloc_value(struct bpf_map_info *info)
return malloc(info->value_size);
}
-static int map_parse_fd(int *argc, char ***argv)
+int map_parse_fd(int *argc, char ***argv)
{
int fd;
@@ -824,7 +824,7 @@ static int do_help(int argc, char **argv)
" %s %s event_pipe MAP [cpu N index M]\n"
" %s %s help\n"
"\n"
- " MAP := { id MAP_ID | pinned FILE }\n"
+ " " HELP_SPEC_MAP "\n"
" DATA := { [hex] BYTES }\n"
" " HELP_SPEC_PROGRAM "\n"
" VALUE := { DATA | MAP | PROG }\n"
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 2bdd5ecd1aad..dce960d22106 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -31,6 +31,7 @@
* SOFTWARE.
*/
+#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <stdarg.h>
@@ -682,18 +683,34 @@ static int do_pin(int argc, char **argv)
return err;
}
+struct map_replace {
+ int idx;
+ int fd;
+ char *name;
+};
+
+int map_replace_compar(const void *p1, const void *p2)
+{
+ const struct map_replace *a = p1, *b = p2;
+
+ return a->idx - b->idx;
+}
+
static int do_load(int argc, char **argv)
{
enum bpf_attach_type expected_attach_type;
struct bpf_object_open_attr attr = {
.prog_type = BPF_PROG_TYPE_UNSPEC,
};
+ struct map_replace *map_replace = NULL;
+ unsigned int old_map_fds = 0;
struct bpf_program *prog;
struct bpf_object *obj;
struct bpf_map *map;
const char *pinfile;
+ unsigned int i, j;
__u32 ifindex = 0;
- int err;
+ int idx, err;
if (!REQ_ARGS(2))
return -1;
@@ -708,16 +725,16 @@ static int do_load(int argc, char **argv)
if (attr.prog_type != BPF_PROG_TYPE_UNSPEC) {
p_err("program type already specified");
- return -1;
+ goto err_free_reuse_maps;
}
if (!REQ_ARGS(1))
- return -1;
+ goto err_free_reuse_maps;
/* Put a '/' at the end of type to appease libbpf */
type = malloc(strlen(*argv) + 2);
if (!type) {
p_err("mem alloc failed");
- return -1;
+ goto err_free_reuse_maps;
}
*type = 0;
strcat(type, *argv);
@@ -728,37 +745,81 @@ static int do_load(int argc, char **argv)
free(type);
if (err < 0) {
p_err("unknown program type '%s'", *argv);
- return err;
+ goto err_free_reuse_maps;
}
NEXT_ARG();
+ } else if (is_prefix(*argv, "map")) {
+ char *endptr, *name;
+ int fd;
+
+ NEXT_ARG();
+
+ if (!REQ_ARGS(4))
+ goto err_free_reuse_maps;
+
+ if (is_prefix(*argv, "idx")) {
+ NEXT_ARG();
+
+ idx = strtoul(*argv, &endptr, 0);
+ if (*endptr) {
+ p_err("can't parse %s as IDX", *argv);
+ goto err_free_reuse_maps;
+ }
+ name = NULL;
+ } else if (is_prefix(*argv, "name")) {
+ NEXT_ARG();
+
+ name = *argv;
+ idx = -1;
+ } else {
+ p_err("expected 'idx' or 'name', got: '%s'?",
+ *argv);
+ goto err_free_reuse_maps;
+ }
+ NEXT_ARG();
+
+ fd = map_parse_fd(&argc, &argv);
+ if (fd < 0)
+ goto err_free_reuse_maps;
+
+ map_replace = reallocarray(map_replace, old_map_fds + 1,
+ sizeof(*map_replace));
+ if (!map_replace) {
+ p_err("mem alloc failed");
+ goto err_free_reuse_maps;
+ }
+ map_replace[old_map_fds].idx = idx;
+ map_replace[old_map_fds].name = name;
+ map_replace[old_map_fds].fd = fd;
+ old_map_fds++;
} else if (is_prefix(*argv, "dev")) {
NEXT_ARG();
if (ifindex) {
p_err("offload device already specified");
- return -1;
+ goto err_free_reuse_maps;
}
if (!REQ_ARGS(1))
- return -1;
+ goto err_free_reuse_maps;
ifindex = if_nametoindex(*argv);
if (!ifindex) {
p_err("unrecognized netdevice '%s': %s",
*argv, strerror(errno));
- return -1;
+ goto err_free_reuse_maps;
}
NEXT_ARG();
} else {
- p_err("expected no more arguments, 'type' or 'dev', got: '%s'?",
+ p_err("expected no more arguments, 'type', 'map' or 'dev', got: '%s'?",
*argv);
- return -1;
+ goto err_free_reuse_maps;
}
}
obj = bpf_object__open_xattr(&attr);
if (IS_ERR_OR_NULL(obj)) {
p_err("failed to open object file");
- return -1;
+ goto err_free_reuse_maps;
}
prog = bpf_program__next(NULL, obj);
@@ -782,10 +843,62 @@ static int do_load(int argc, char **argv)
bpf_program__set_type(prog, attr.prog_type);
bpf_program__set_expected_attach_type(prog, expected_attach_type);
- bpf_map__for_each(map, obj)
+ qsort(map_replace, old_map_fds, sizeof(*map_replace),
+ map_replace_compar);
+
+ /* After the sort maps by name will be first on the list, because they
+ * have idx == -1. Resolve them.
+ */
+ j = 0;
+ while (j < old_map_fds && map_replace[j].name) {
+ i = 0;
+ bpf_map__for_each(map, obj) {
+ if (!strcmp(bpf_map__name(map), map_replace[j].name)) {
+ map_replace[j].idx = i;
+ break;
+ }
+ i++;
+ }
+ if (map_replace[j].idx == -1) {
+ p_err("unable to find map '%s'", map_replace[j].name);
+ goto err_close_obj;
+ }
+ j++;
+ }
+ /* Resort if any names were resolved */
+ if (j)
+ qsort(map_replace, old_map_fds, sizeof(*map_replace),
+ map_replace_compar);
+
+ /* Set ifindex and name reuse */
+ j = 0;
+ idx = 0;
+ bpf_map__for_each(map, obj) {
if (!bpf_map__is_offload_neutral(map))
bpf_map__set_ifindex(map, ifindex);
+ if (j < old_map_fds && idx == map_replace[j].idx) {
+ err = bpf_map__reuse_fd(map, map_replace[j++].fd);
+ if (err) {
+ p_err("unable to set up map reuse: %d", err);
+ goto err_close_obj;
+ }
+
+ /* Next reuse wants to apply to the same map */
+ if (j < old_map_fds && map_replace[j].idx == idx) {
+ p_err("replacement for map idx %d specified more than once",
+ idx);
+ goto err_close_obj;
+ }
+ }
+
+ idx++;
+ }
+ if (j < old_map_fds) {
+ p_err("map idx '%d' not used", map_replace[j].idx);
+ goto err_close_obj;
+ }
+
err = bpf_object__load(obj);
if (err) {
p_err("failed to load object file");
@@ -799,11 +912,18 @@ static int do_load(int argc, char **argv)
jsonw_null(json_wtr);
bpf_object__close(obj);
+ for (i = 0; i < old_map_fds; i++)
+ close(map_replace[i].fd);
+ free(map_replace);
return 0;
err_close_obj:
bpf_object__close(obj);
+err_free_reuse_maps:
+ for (i = 0; i < old_map_fds; i++)
+ close(map_replace[i].fd);
+ free(map_replace);
return -1;
}
@@ -819,9 +939,11 @@ static int do_help(int argc, char **argv)
" %s %s dump xlated PROG [{ file FILE | opcodes | visual }]\n"
" %s %s dump jited PROG [{ file FILE | opcodes }]\n"
" %s %s pin PROG FILE\n"
- " %s %s load OBJ FILE [type TYPE] [dev NAME]\n"
+ " %s %s load OBJ FILE [type TYPE] [dev NAME] \\\n"
+ " [map { idx IDX | name NAME } MAP]\n"
" %s %s help\n"
"\n"
+ " " HELP_SPEC_MAP "\n"
" " HELP_SPEC_PROGRAM "\n"
" TYPE := { socket | kprobe | kretprobe | classifier | action |\n"
" tracepoint | raw_tracepoint | xdp | perf_event | cgroup/skb |\n"
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next v2 02/10] r8169: use phy_resume/phy_suspend
From: Heiner Kallweit @ 2018-07-10 21:56 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <20180710205247.GI892@lunn.ch>
On 10.07.2018 22:52, Andrew Lunn wrote:
>>> Why is it powered up, but not connected? Is it powered down before it
>>> is disconnected? Is it the bootloader which is powering it up?
>>>
>> Exactly, if the device is active when driver is loaded and the
>> interface isn't used and therefore not brought up, then, when runtime-
>> suspending, we face this situation.
>
> Well, it is more complex than that. If the interface is not used, why
> bother even loading the MAC driver?
>
> If you are trying to make a really low power system, the bootloader
> also needs to be involved. It needs to shut down anything it starts
> before handing over control the linux.
>
> Another approach is to handle this in phylib. When the mdio bus is
> suspended, look for any PHYs which are not connected and power them
> down. The assumption being, any MAC driver using WoL via a PHY does
> not disconnect the PHY before suspending.
>
Tricky part may be the differentiation between system- and runtime-
suspend. IIRC the MDIO bus doesn't have runtime pm ops (yet) and
most likely we would have to add them.
> Andrew
>
^ permalink raw reply
* Re: [PATCH net-next 01/13] ARM: net: bpf: enumerate the JIT scratch stack layout
From: Russell King - ARM Linux @ 2018-07-10 21:57 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, linux-arm-kernel
In-Reply-To: <41b5fc68-e15f-56f9-c55b-4f6deb61c54c@iogearbox.net>
On Tue, Jul 10, 2018 at 08:30:04PM +0200, Daniel Borkmann wrote:
> Hi Russell,
>
> thanks a lot for your work on the arm32 JIT!
>
> On 07/10/2018 02:36 PM, Russell King wrote:
> > Enumerate the contents of the JIT scratch stack layout used for storing
> > some of the JITs 64-bit registers, tail call counter and AX register.
> >
> > XXX: what about the skb_copy_bits buffer - this appears to overlap with
> > the first word of the JITs accessible stack.
>
> Could you elaborate on that case? Unless I'm missing something there should
> be no use of the skb_copy_bits buffer anymore (aka former SKB_BUFFER at
> STACK_VAR(SCRATCH_SIZE) offset), but aside from that it's not supposed to
> overlap either.
Probably an old comment - these were originally developed back in
January timeframe when there was the SKB_BUFFER stuff, but that was
removed during the 4.18 merge window. I'll kill the comment.
Thanks.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up
^ permalink raw reply
* Re: [PATCH mlx5-next 2/9] net/mlx5: Add support for flow table destination number
From: Saeed Mahameed @ 2018-07-10 22:26 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Jason Gunthorpe, Leon Romanovsky, RDMA mailing list,
Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <20180708102445.25496-3-leon@kernel.org>
On Sun, Jul 8, 2018 at 3:24 AM, Leon Romanovsky <leon@kernel.org> wrote:
> From: Yishai Hadas <yishaih@mellanox.com>
>
> Add support to set a destination from a flow table number.
> This functionality will be used in downstream patches from this
> series by the DEVX stuff.
>
> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> .../mellanox/mlx5/core/diag/fs_tracepoint.c | 3 +++
> drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c | 21 ++++++++++++++-------
> drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 4 +++-
> include/linux/mlx5/fs.h | 1 +
> include/linux/mlx5/mlx5_ifc.h | 1 +
> 5 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c b/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c
> index b3820a34e773..0f11fff32a9b 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c
> @@ -240,6 +240,9 @@ const char *parse_fs_dst(struct trace_seq *p,
> case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE:
> trace_seq_printf(p, "ft=%p\n", dst->ft);
> break;
> + case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM:
> + trace_seq_printf(p, "ft_num=%u\n", dst->ft_num);
> + break;
> case MLX5_FLOW_DESTINATION_TYPE_TIR:
> trace_seq_printf(p, "tir=%u\n", dst->tir_num);
> break;
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
> index 5a00deff5457..a98584576c2e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
> @@ -363,17 +363,21 @@ static int mlx5_cmd_set_fte(struct mlx5_core_dev *dev,
>
> list_for_each_entry(dst, &fte->node.children, node.list) {
> unsigned int id;
> + unsigned int type;
>
reversed xmas tree, preferably just do:
unsigned int id, type = dst->dest_attr.type;
// use type directly instead of dst->dest_attr.type below ..
> if (dst->dest_attr.type == MLX5_FLOW_DESTINATION_TYPE_COUNTER)
> continue;
>
> - MLX5_SET(dest_format_struct, in_dests, destination_type,
> - dst->dest_attr.type);
> - if (dst->dest_attr.type ==
> - MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE) {
> + type = dst->dest_attr.type;
> + switch (type) {
> + case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM:
> + id = dst->dest_attr.ft_num;
> + type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
> + break;
> + case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE:
> id = dst->dest_attr.ft->id;
> - } else if (dst->dest_attr.type ==
> - MLX5_FLOW_DESTINATION_TYPE_VPORT) {
> + break;
> + case MLX5_FLOW_DESTINATION_TYPE_VPORT:
> id = dst->dest_attr.vport.num;
> MLX5_SET(dest_format_struct, in_dests,
> destination_eswitch_owner_vhca_id_valid,
> @@ -381,9 +385,12 @@ static int mlx5_cmd_set_fte(struct mlx5_core_dev *dev,
> MLX5_SET(dest_format_struct, in_dests,
> destination_eswitch_owner_vhca_id,
> dst->dest_attr.vport.vhca_id);
> - } else {
> + break;
> + default:
> id = dst->dest_attr.tir_num;
> }
> +
> + MLX5_SET(dest_format_struct, in_dests, destination_type, type);
> MLX5_SET(dest_format_struct, in_dests, destination_id, id);
> in_dests += MLX5_ST_SZ_BYTES(dest_format_struct);
> list_size++;
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> index eba113cf1117..69aa298a0b1c 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> @@ -1356,7 +1356,9 @@ static bool mlx5_flow_dests_cmp(struct mlx5_flow_destination *d1,
> (d1->type == MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE &&
> d1->ft == d2->ft) ||
> (d1->type == MLX5_FLOW_DESTINATION_TYPE_TIR &&
> - d1->tir_num == d2->tir_num))
> + d1->tir_num == d2->tir_num) ||
> + (d1->type == MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM &&
> + d1->ft_num == d2->ft_num))
> return true;
> }
>
> diff --git a/include/linux/mlx5/fs.h b/include/linux/mlx5/fs.h
> index 757b4a30281e..a45febcf6b51 100644
> --- a/include/linux/mlx5/fs.h
> +++ b/include/linux/mlx5/fs.h
> @@ -89,6 +89,7 @@ struct mlx5_flow_destination {
> enum mlx5_flow_destination_type type;
> union {
> u32 tir_num;
> + u32 ft_num;
> struct mlx5_flow_table *ft;
> struct mlx5_fc *counter;
> struct {
> diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
> index 44a6ce01c3bb..fb89cc519703 100644
> --- a/include/linux/mlx5/mlx5_ifc.h
> +++ b/include/linux/mlx5/mlx5_ifc.h
> @@ -1181,6 +1181,7 @@ enum mlx5_flow_destination_type {
>
> MLX5_FLOW_DESTINATION_TYPE_PORT = 0x99,
> MLX5_FLOW_DESTINATION_TYPE_COUNTER = 0x100,
> + MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM = 0x101,
> };
>
> struct mlx5_ifc_dest_format_struct_bits {
> --
> 2.14.4
>
^ permalink raw reply
* Re: [PATCH bpf v2 1/1] bpf: btf: Fix bitfield extraction for big endian
From: Martin KaFai Lau @ 2018-07-10 23:46 UTC (permalink / raw)
To: Okash Khawaja
Cc: Daniel Borkmann, Alexei Starovoitov, Yonghong Song,
Jakub Kicinski, David S. Miller, netdev, kernel-team,
linux-kernel
In-Reply-To: <20180710214522.339195416@fb.com>
On Tue, Jul 10, 2018 at 02:33:07PM -0700, Okash Khawaja wrote:
> When extracting bitfield from a number, btf_int_bits_seq_show() builds
> a mask and accesses least significant byte of the number in a way
> specific to little-endian. This patch fixes that by checking endianness
> of the machine and then shifting left and right the unneeded bits.
>
> Thanks to Martin Lau for the help in navigating potential pitfalls when
> dealing with endianess and for the final solution.
>
> Fixes: b00b8daec828 ("bpf: btf: Add pretty print capability for data with BTF type info")
> Signed-off-by: Okash Khawaja <osk@fb.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply
* Re: [RFC] Add BPF_SYNCHRONIZE bpf(2) command
From: Alexei Starovoitov @ 2018-07-10 23:52 UTC (permalink / raw)
To: Chenbo Feng
Cc: Daniel Colascione, mathieu.desnoyers, Joel Fernandes,
Alexei Starovoitov, linux-kernel, timmurray, Daniel Borkmann,
netdev, Lorenzo Colitti
In-Reply-To: <CAMOXUJ=QmkjTqbWqPRxb=R7f6EpsHBkDQ4MhrSD=VPiDTj7RiA@mail.gmail.com>
On Mon, Jul 09, 2018 at 10:25:04PM -0700, Chenbo Feng wrote:
> On Mon, Jul 9, 2018 at 3:34 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Mon, Jul 09, 2018 at 03:21:43PM -0700, Daniel Colascione wrote:
> > > On Mon, Jul 9, 2018 at 3:10 PM, Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > > On Mon, Jul 09, 2018 at 02:36:37PM -0700, Daniel Colascione wrote:
> > > >> On Mon, Jul 9, 2018 at 2:09 PM, Alexei Starovoitov
> > > >> <alexei.starovoitov@gmail.com> wrote:
> > > >> > On Sun, Jul 08, 2018 at 04:54:38PM -0400, Mathieu Desnoyers wrote:
> > > >> >> ----- On Jul 7, 2018, at 4:33 PM, Joel Fernandes joelaf@google.com wrote:
> > > >> >>
> > > >> >> > On Fri, Jul 06, 2018 at 07:54:28PM -0700, Alexei Starovoitov wrote:
> > > >> >> >> On Fri, Jul 06, 2018 at 06:56:16PM -0700, Daniel Colascione wrote:
> > > >> >> >> > BPF_SYNCHRONIZE waits for any BPF programs active at the time of
> > > >> >> >> > BPF_SYNCHRONIZE to complete, allowing userspace to ensure atomicity of
> > > >> >> >> > RCU data structure operations with respect to active programs. For
> > > >> >> >> > example, userspace can update a map->map entry to point to a new map,
> > > >> >> >> > use BPF_SYNCHRONIZE to wait for any BPF programs using the old map to
> > > >> >> >> > complete, and then drain the old map without fear that BPF programs
> > > >> >> >> > may still be updating it.
> > > >> >> >> >
> > > >> >> >> > Signed-off-by: Daniel Colascione <dancol@google.com>
> > > >> >> >> > ---
> > > >> >> >> > include/uapi/linux/bpf.h | 1 +
> > > >> >> >> > kernel/bpf/syscall.c | 14 ++++++++++++++
> > > >> >> >> > 2 files changed, 15 insertions(+)
> > > >> >> >> >
> > > >> >> >> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > >> >> >> > index b7db3261c62d..4365c50e8055 100644
> > > >> >> >> > --- a/include/uapi/linux/bpf.h
> > > >> >> >> > +++ b/include/uapi/linux/bpf.h
> > > >> >> >> > @@ -98,6 +98,7 @@ enum bpf_cmd {
> > > >> >> >> > BPF_BTF_LOAD,
> > > >> >> >> > BPF_BTF_GET_FD_BY_ID,
> > > >> >> >> > BPF_TASK_FD_QUERY,
> > > >> >> >> > + BPF_SYNCHRONIZE,
> > > >> >> >> > };
> > > >> >> >> >
> > > >> >> >> > enum bpf_map_type {
> > > >> >> >> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > > >> >> >> > index d10ecd78105f..60ec7811846e 100644
> > > >> >> >> > --- a/kernel/bpf/syscall.c
> > > >> >> >> > +++ b/kernel/bpf/syscall.c
> > > >> >> >> > @@ -2272,6 +2272,20 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *,
> > > >> >> >> > uattr, unsigned int, siz
> > > >> >> >> > if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))
> > > >> >> >> > return -EPERM;
> > > >> >> >> >
> > > >> >> >> > + if (cmd == BPF_SYNCHRONIZE) {
> > > >> >> >> > + if (uattr != NULL || size != 0)
> > > >> >> >> > + return -EINVAL;
> > > >> >> >> > + err = security_bpf(cmd, NULL, 0);
> > > >> >> >> > + if (err < 0)
> > > >> >> >> > + return err;
> > > >> >> >> > + /* BPF programs are run with preempt disabled, so
> > > >> >> >> > + * synchronize_sched is sufficient even with
> > > >> >> >> > + * RCU_PREEMPT.
> > > >> >> >> > + */
> > > >> >> >> > + synchronize_sched();
> > > >> >> >> > + return 0;
> > > >> >> >>
> > > >> >> >> I don't think it's necessary. sys_membarrier() can do this already
> > > >> >> >> and some folks use it exactly for this use case.
> > > >> >> >
> > > >> >> > Alexei, the use of sys_membarrier for this purpose seems kind of weird to me
> > > >> >> > though. No where does the manpage say membarrier should be implemented this
> > > >> >> > way so what happens if the implementation changes?
> > > >> >> >
> > > >> >> > Further, membarrier manpage says that a memory barrier should be matched with
> > > >> >> > a matching barrier. In this use case there is no matching barrier, so it
> > > >> >> > makes it weirder.
> > > >> >> >
> > > >> >> > Lastly, sys_membarrier seems will not work on nohz-full systems, so its a bit
> > > >> >> > fragile to depend on it for this?
> > > >> >> >
> > > >> >> > case MEMBARRIER_CMD_GLOBAL:
> > > >> >> > /* MEMBARRIER_CMD_GLOBAL is not compatible with nohz_full. */
> > > >> >> > if (tick_nohz_full_enabled())
> > > >> >> > return -EINVAL;
> > > >> >> > if (num_online_cpus() > 1)
> > > >> >> > synchronize_sched();
> > > >> >> > return 0;
> > > >> >> >
> > > >> >> >
> > > >> >> > Adding Mathieu as well who I believe is author/maintainer of membarrier.
> > > >> >>
> > > >> >> See commit 907565337
> > > >> >> "Fix: Disable sys_membarrier when nohz_full is enabled"
> > > >> >>
> > > >> >> "Userspace applications should be allowed to expect the membarrier system
> > > >> >> call with MEMBARRIER_CMD_SHARED command to issue memory barriers on
> > > >> >> nohz_full CPUs, but synchronize_sched() does not take those into
> > > >> >> account."
> > > >> >>
> > > >> >> So AFAIU you'd want to re-use membarrier to issue synchronize_sched, and you
> > > >> >> only care about kernel preempt off critical sections.
> > > >> >>
> > > >> >> Clearly bpf code does not run in user-space, so it would "work".
> > > >> >>
> > > >> >> But the guarantees provided by membarrier are not to synchronize against
> > > >> >> preempt off per se. It's just that the current implementation happens to
> > > >> >> do that. The point of membarrier is to turn user-space memory barriers
> > > >> >> into compiler barriers.
> > > >> >>
> > > >> >> If what you need is to wait for a RCU grace period for whatever RCU flavor
> > > >> >> ebpf is using, I would against using membarrier for this. I would rather
> > > >> >> recommend adding a dedicated BPF_SYNCHRONIZE so you won't leak
> > > >> >> implementation details to user-space, *and* you can eventually change you
> > > >> >> RCU implementation for e.g. SRCU in the future if needed.
> > > >> >
> > > >> > The point about future changes to underlying bpf mechanisms is valid.
> > > >> > There is work already on the way to reduce the scope of preempt_off+rcu_lock
> > > >> > that currently lasts the whole prog. We will have new prog types that won't
> > > >> > have such wrappers and will do rcu_lock/unlock and preempt on/off only
> > > >> > when necessary.
> > > >> > So something like BPF_SYNCHRONIZE will break soon, since the kernel cannot have
> > > >> > guarantees on when programs finish. Calling this command BPF_SYNCHRONIZE_PROG
> > > >> > also won't make sense for the same reason.
> > > >> > What we can do it instead is to define synchronization barrier for
> > > >> > programs accessing maps. May be call it something like:
> > > >> > BPF_SYNC_MAP_ACCESS ?
> > > >>
> > > >> I'm not sure what you're proposing. In the case the commit message
> > > >> describes, a user-space program that wants to "drain" a map needs to
> > > >> be confident that the map won't change under it, even across multiple
> > > >> bpf system calls on that map. One way of doing that is to ensure that
> > > >> nothing that could possibly hold a reference to that map is still
> > > >> running. Are you proposing some kind of refcount-draining approach?
> > > >> Simple locking won't work, since BPF programs can't block, and I don't
> > > >> see right now how a simple barrier would help.
> > > >
> > > > I'm proposing few changes for your patch:
> > > > s/BPF_SYNCHRONIZE/BPF_SYNC_MAP_ACCESS/
> > > > and s/synchronize_sched/synchronize_rcu/
> > > > with detailed comment in uapi/bpf.h that has an example why folks
> > > > would want to use this new cmd.
> > >
> > > Thanks for clarifying.
> > >
> > > > I think the bpf maps will be rcu protected for foreseeable future
> > > > even when rcu_read_lock/unlock will be done by the programs instead of
> > > > kernel wrappers.
> > >
> > > Can we guarantee that we always obtain a map reference and dispose of
> > > that reference inside the same critical section?
> >
> > yep. the verifier will guarantee that.
> >
> > > If so, can BPF
> > > programs then disable preemption for as long as they'd like?
> >
> > you mean after the finish? no. only while running.
> > The verifier will match things like lookup/release, lock/unlock, preempt on/off
> > and will make sure there is no dangling preempt disable after program returns.
> >
> It might be a silly question but does this new bpf program type
> provide a way to customize where to hold the rcu_lock/unlock and
> enable/disable preemption when creating the program? Since the
> BPF_SYNC_MAP_ACCESS cmd sounds not very useful if it only protect each
> single map access instead of the whole program. For example, sometimes
> we have to read the same map multiple times when running a eBPF
> program and if the userspace changed the map value between two reads,
> it may cause troubles. It would be great if we can have a RCU lock on
> a block of eBPF program and in that way I think BPF_SYNC_MAP_ACCESS
> cmd should be enough in handling userspace kernel racing problems.
we need to make sure we have detailed description of BPF_SYNC_MAP_ACCESS
in uapi/bpf.h, since I feel the confusion regarding its usage is starting already.
This new cmd will only make sense for map-in-map type of maps.
Expecting that BPF_SYNC_MAP_ACCESS is somehow implies the end of
the program or doing some other map synchronization is not correct.
Commit log of this patch got it right:
"""
For example, userspace can update a map->map entry to point to a new map,
use BPF_SYNCHRONIZE to wait for any BPF programs using the old map to
complete, and then drain the old map without fear that BPF programs
may still be updating it.
"""
Now consider two cases:
1. single program does two lookups (inner and outer map) and the program is called
twice (as trigger on two events or it was tail-called or through prog_array)
2. future single program without rcu wrappers does:
rcu_lock + outer lookup + inner lookup + rcu_unlock + .. some other bpf stuff .. +
rcu_lock + outer lookup + inner lookup + rcu_unlock + .. yet another bpf stuff ..
All of the above as part of single program that is called once.
These two cases are the same from the point of view of BPF_SYNC_MAP_ACCESS
and completion of this cmd does not guarantee that program finished.
In both cases the user space will be correct to start draining the inner map
it replaced after BPF_SYNC_MAP_ACCESS is done
(assuming that implementation does synchronize_rcu I mentioned earlier)
Preemption on/off as done today by prog wrappers or in the future explicitly
by the prog code is orthogonal to this new cmd and map-in-map draining.
^ permalink raw reply
* Re: [PATCH 2/3] rhashtable: add rhashtable_walk_last_seen()
From: David Miller @ 2018-07-10 23:55 UTC (permalink / raw)
To: neilb; +Cc: tgraf, herbert, tom, netdev, linux-kernel
In-Reply-To: <153086109260.2825.1459722786871837620.stgit@noble>
From: NeilBrown <neilb@suse.com>
Date: Fri, 06 Jul 2018 17:11:32 +1000
> rhashtable_walk_last_seen() returns the object returned by
> the previous rhashtable_walk_next(), providing it is still in the
> table (or was during this grace period).
> This works even if rhashtable_walk_stop() and rhashtable_talk_start()
^^^^
"walk"
^ permalink raw reply
* Re: [PATCH 0/3] rhashtable: replace rhashtable_walk_peek implementation
From: David Miller @ 2018-07-10 23:55 UTC (permalink / raw)
To: neilb; +Cc: tgraf, herbert, tom, netdev, linux-kernel
In-Reply-To: <153086101070.2825.6850140624411927465.stgit@noble>
From: NeilBrown <neilb@suse.com>
Date: Fri, 06 Jul 2018 17:11:32 +1000
> This is a resend of these three patches with no change
> from last time.
> The last patch has an Ack from Tom Herbert and Herbert Xu, but the
> first two which are necessary pre-requisites don't yet.
Please repost this when your dependencies are actually in the
tree, not beforehand.
Otherwise you make more work for me, and you completely destroy
the value of the kbuild test robot.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next 0/2] net/sched: act_skbedit: lockless data path
From: David Miller @ 2018-07-10 23:58 UTC (permalink / raw)
To: dcaratti; +Cc: xiyou.wangcong, netdev
In-Reply-To: <cover.1530870765.git.dcaratti@redhat.com>
From: Davide Caratti <dcaratti@redhat.com>
Date: Fri, 6 Jul 2018 12:40:12 +0200
> the data path of act_skbedit can be faster if we avoid using spinlocks:
> - patch 1 converts act_skbedit statistics to use per-cpu counters
> - patch 2 lets act_skbedit use RCU to read/update its configuration
This doesn't apply cleanly to net-next, please respin.
Thank you.
^ permalink raw reply
* Re: Re: [Qemu-devel] [PATCH v3 0/3] Use of unique identifier for pairing virtio and passthrough devices...
From: Siwei Liu @ 2018-07-11 0:07 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: si-wei liu, Roman Kagan, Venu Busireddy, Marcel Apfelbaum,
virtio-dev, qemu-devel, Samudrala, Sridhar, Alexander Duyck,
Netdev
In-Reply-To: <20180710045101-mutt-send-email-mst@kernel.org>
On Mon, Jul 9, 2018 at 6:54 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Mon, Jul 09, 2018 at 06:11:53PM -0700, si-wei liu wrote:
>> The plan is to enable group ID based matching in the first place rather than
>> match by MAC, the latter of which is fragile and problematic.
>
> It isn't all that fragile - hyperv used same for a while, so if someone
> posts working patches with QEMU support but before this grouping stuff,
> I'll happily apply them.
I wouldn't box the solution to very limited scenario just because of
matching by MAC, the benefit of having generic group ID in the first
place is that we save the effort of maintaining legacy MAC based
pairing that just adds complexity anyway. Currently the VF's MAC
address cannot be changed by either PF or by the guest user is a
severe limitation due to this. The other use case is that PT device
than VF would generally have different MAC than the standby virtio. We
shouldn't limit itself to VF specific scenario from the very
beginning.
It's just small piece of QEMU code to add in, why not?
>
>> I have made
>> the Linux side changes and will get it posted once the QEMU discussion for
>> grouping is finalized.
>
> Go ahead but at this point I think we need to see actual QEMU support at
> least for the basic functionality before we merge more code Linux-side.
The VFIO visibility work is being worked on. We'll post patches when it's ready.
Thanks,
-Siwei
>
> --
> MST
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>
^ permalink raw reply
* Re: [net-next,v3] tcp: Improve setsockopt() TCP_USER_TIMEOUT accuracy
From: Jonathan Maxwell @ 2018-07-11 0:33 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Eric Dumazet, Neal Cardwell, David Laight, kuznet,
yoshfuji, Netdev, LKML, Jon Maxwell
In-Reply-To: <c29f1535-3c5d-e588-862c-ba948363c973@gmail.com>
On Tue, Jul 10, 2018 at 10:48 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> On 07/10/2018 05:38 AM, Eric Dumazet wrote:
>
>> Note that if we always do jiffies_to_msecs(icsk->icsk_user_timeout) in TCP,
>> we also could change the convention and store msecs in this field instead of jiffies.
>>
>> That would eliminate the msecs_to_jiffies() and jiffies_to_msecs() dance.
>>
>> (That would be done in a patch of its own, of course)
>
> tcp_keepalive_timer() does use icsk->icsk_user_timeout directly in jiffies unit,
> but considering keeapalive timers are rarely used, this point would have to
> do the msecs_to_jiffies() conversion.
and also if icsk->icsk_user_timeout = 0, then timeout in retransmits_timed_out()
is in jiffies and that would need to addressed.
^ permalink raw reply
* Re: [PATCH net-next v1 5/5] virtio_ring: enable packed ring
From: Tiwei Bie @ 2018-07-11 1:07 UTC (permalink / raw)
To: Jason Wang; +Cc: virtio-dev, mst, netdev, linux-kernel, virtualization, wexu
In-Reply-To: <27ae72ad-247b-e8f0-aa37-a75a719e0e01@redhat.com>
On Tue, Jul 10, 2018 at 01:51:20PM +0800, Jason Wang wrote:
> On 2018年07月09日 15:22, Tiwei Bie wrote:
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > ---
> > drivers/s390/virtio/virtio_ccw.c | 8 ++++++++
> > drivers/virtio/virtio_ring.c | 2 ++
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
> > index 8f5c1d7f751a..ff5b85736d8d 100644
> > --- a/drivers/s390/virtio/virtio_ccw.c
> > +++ b/drivers/s390/virtio/virtio_ccw.c
> > @@ -765,6 +765,11 @@ static u64 virtio_ccw_get_features(struct virtio_device *vdev)
> > return rc;
> > }
> > +static void ccw_transport_features(struct virtio_device *vdev)
> > +{
> > + __virtio_clear_bit(vdev, VIRTIO_F_RING_PACKED);
> > +}
>
> I think we need a better comment to explain why it was disabled here.
Yeah, I'll do it!
Best regards,
Tiwei Bie
>
> Thanks
>
> > +
> > static int virtio_ccw_finalize_features(struct virtio_device *vdev)
> > {
> > struct virtio_ccw_device *vcdev = to_vc_device(vdev);
> > @@ -791,6 +796,9 @@ static int virtio_ccw_finalize_features(struct virtio_device *vdev)
> > /* Give virtio_ring a chance to accept features. */
> > vring_transport_features(vdev);
> > + /* Give virtio_ccw a chance to accept features. */
> > + ccw_transport_features(vdev);
> > +
> > features->index = 0;
> > features->features = cpu_to_le32((u32)vdev->features);
> > /* Write the first half of the feature bits to the host. */
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 4b3f9e1a3cab..64f20023f088 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -1968,6 +1968,8 @@ void vring_transport_features(struct virtio_device *vdev)
> > break;
> > case VIRTIO_F_IOMMU_PLATFORM:
> > break;
> > + case VIRTIO_F_RING_PACKED:
> > + break;
> > default:
> > /* We don't understand this bit. */
> > __virtio_clear_bit(vdev, i);
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [virtio-dev] Re: [PATCH net-next v1 4/5] virtio_ring: add event idx support in packed ring
From: Tiwei Bie @ 2018-07-11 1:09 UTC (permalink / raw)
To: Jason Wang
Cc: mst, virtualization, linux-kernel, netdev, virtio-dev, wexu,
jfreimann
In-Reply-To: <9ee31221-9f4a-f1c5-0f0b-8d178bb536e9@redhat.com>
On Tue, Jul 10, 2018 at 01:50:03PM +0800, Jason Wang wrote:
> On 2018年07月09日 15:22, Tiwei Bie wrote:
> > @@ -1059,9 +1059,19 @@ static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
> > * suppressions. */
> > virtio_mb(vq->weak_barriers);
> > + old = vq->next_avail_idx - vq->num_added;
> > + new = vq->next_avail_idx;
> > + vq->num_added = 0;
> > +
> > snapshot = *(u32 *)vq->vring_packed.device;
>
> I think we should use READ_ONCE() to prevent compiler from re-reading.
I'll do it. Thanks!
Best regards,
Tiwei Bie
>
> > + off_wrap = virtio16_to_cpu(_vq->vdev, (__virtio16)(snapshot & 0xffff));
> > flags = virtio16_to_cpu(_vq->vdev, (__virtio16)(snapshot >> 16)) & 0x3;
> > + wrap_counter = off_wrap >> 15;
> > + event_idx = off_wrap & ~(1 << 15);
> > + if (wrap_counter != vq->avail_wrap_counter)
> > + event_idx -= vq->vring_packed.num;
>
> Thanks
>
^ permalink raw reply
* [PATCH net-next] tc-testing: add geneve options in tunnel_key unit tests
From: Jakub Kicinski @ 2018-07-11 1:22 UTC (permalink / raw)
To: davem
Cc: Keara Leibovitz, Roman Mashak, Lucas Bates, oss-drivers, netdev,
Pieter Jansen van Vuuren
From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Extend tc tunnel_key action unit tests with geneve options. Tests
include testing single and multiple geneve options, as well as
testing geneve options that are expected to fail.
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
---
.../tc-tests/actions/tunnel_key.json | 168 ++++++++++++++++++
1 file changed, 168 insertions(+)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json b/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
index d878ce1c7d08..10b2d894e436 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
@@ -640,6 +640,174 @@
"$TC actions flush action tunnel_key"
]
},
+ {
+ "id": "4f20",
+ "name": "Add tunnel_key action with a single geneve option parameter",
+ "category": [
+ "actions",
+ "tunnel_key"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action tunnel_key",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 0102:80:00880022 index 1",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions get action tunnel_key index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 0102:80:00880022.*index 1",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action tunnel_key"
+ ]
+ },
+ {
+ "id": "e33d",
+ "name": "Add tunnel_key action with multiple geneve options parameter",
+ "category": [
+ "actions",
+ "tunnel_key"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action tunnel_key",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 0102:80:00880022,0408:42:0040007611223344,0111:02:1020304011223344 index 1",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions get action tunnel_key index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 0102:80:00880022,0408:42:0040007611223344,0111:02:1020304011223344.*index 1",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action tunnel_key"
+ ]
+ },
+ {
+ "id": "0778",
+ "name": "Add tunnel_key action with invalid class geneve option parameter",
+ "category": [
+ "actions",
+ "tunnel_key"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action tunnel_key",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 824212:80:00880022 index 1",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions get action tunnel_key index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 824212:80:00880022.*index 1",
+ "matchCount": "0",
+ "teardown": [
+ "$TC actions flush action tunnel_key"
+ ]
+ },
+ {
+ "id": "4ae8",
+ "name": "Add tunnel_key action with invalid type geneve option parameter",
+ "category": [
+ "actions",
+ "tunnel_key"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action tunnel_key",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 0102:4224:00880022 index 1",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions get action tunnel_key index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 0102:4224:00880022.*index 1",
+ "matchCount": "0",
+ "teardown": [
+ "$TC actions flush action tunnel_key"
+ ]
+ },
+ {
+ "id": "4039",
+ "name": "Add tunnel_key action with short data length geneve option parameter",
+ "category": [
+ "actions",
+ "tunnel_key"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action tunnel_key",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 0102:80:4288 index 1",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions get action tunnel_key index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 0102:80:4288.*index 1",
+ "matchCount": "0",
+ "teardown": [
+ "$TC actions flush action tunnel_key"
+ ]
+ },
+ {
+ "id": "26a6",
+ "name": "Add tunnel_key action with non-multiple of 4 data length geneve option parameter",
+ "category": [
+ "actions",
+ "tunnel_key"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action tunnel_key",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 0102:80:4288428822 index 1",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions get action tunnel_key index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 0102:80:4288428822.*index 1",
+ "matchCount": "0",
+ "teardown": [
+ "$TC actions flush action tunnel_key"
+ ]
+ },
+ {
+ "id": "f44d",
+ "name": "Add tunnel_key action with incomplete geneve options parameter",
+ "category": [
+ "actions",
+ "tunnel_key"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action tunnel_key",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 0102:80:00880022,0408:42: index 1",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions get action tunnel_key index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 0102:80:00880022,0408:42:.*index 1",
+ "matchCount": "0",
+ "teardown": [
+ "$TC actions flush action tunnel_key"
+ ]
+ },
{
"id": "7afc",
"name": "Replace tunnel_key set action with all parameters",
--
2.17.1
^ permalink raw reply related
* Re: [V9fs-developer] [PATCH] version pointer uninitialized
From: jiangyiwen @ 2018-07-11 1:26 UTC (permalink / raw)
To: Tomas Bortoli, ericvh, rminnich, lucho
Cc: netdev, linux-kernel, syzkaller, v9fs-developer, davem
In-Reply-To: <20180709222943.19503-1-tomasbortoli@gmail.com>
On 2018/7/10 6:29, Tomas Bortoli wrote:
> The p9_client_version() does not initialize the version
> pointer. If the call to p9pdu_readf() returns an error and version has not
> been allocated in p9pdu_readf(), then the program will jump to the "error"
> label and will try to free the version pointer. If version is not
> initialized, free() will be called with uninitialized, garbage data and
> will provoke a crash.
>
> Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
Reviewed-by: Yiwen Jiang <jiangyiwen@huawei.com>
> Reported-by: syzbot+65c6b72f284a39d416b4@syzkaller.appspotmail.com
> ---
> net/9p/client.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 18c5271910dc..40f7c47f2f74 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -957,7 +957,7 @@ static int p9_client_version(struct p9_client *c)
> {
> int err = 0;
> struct p9_req_t *req;
> - char *version;
> + char *version = NULL;
> int msize;
>
> p9_debug(P9_DEBUG_9P, ">>> TVERSION msize %d protocol %d\n",
>
^ permalink raw reply
* Re: [PATCH v2 net-next 00/18] TLS offload rx, netdev & mlx5
From: Boris Pismenny @ 2018-07-11 1:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev, davejwatson, aviadye, saeedm
In-Reply-To: <20180709.162307.164922546753947325.davem@davemloft.net>
On 7/9/2018 7:23 PM, David Miller wrote:
> From: Boris Pismenny <borisp@mellanox.com>
> Date: Sun, 8 Jul 2018 12:36:14 +0300
>
>> The following series provides TLS TX inline crypto offload.
>
> I think this patch series provides RX inline offload not TX inline
> offload.
>
Right, sorry for confusing the 2 in this statement above.
^ permalink raw reply
* Re: [net-next,v3] tcp: Improve setsockopt() TCP_USER_TIMEOUT accuracy
From: Eric Dumazet @ 2018-07-11 2:04 UTC (permalink / raw)
To: Jonathan Maxwell, Eric Dumazet
Cc: David Miller, Eric Dumazet, Neal Cardwell, David Laight, kuznet,
yoshfuji, Netdev, LKML, Jon Maxwell
In-Reply-To: <CAGHK07Aoa81QN+LmVttJQ56Sn+R-KyksYqZxSYL5-5reDRt3WA@mail.gmail.com>
On 07/10/2018 05:33 PM, Jonathan Maxwell wrote:
> On Tue, Jul 10, 2018 at 10:48 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>>
>> On 07/10/2018 05:38 AM, Eric Dumazet wrote:
>>
>>> Note that if we always do jiffies_to_msecs(icsk->icsk_user_timeout) in TCP,
>>> we also could change the convention and store msecs in this field instead of jiffies.
>>>
>>> That would eliminate the msecs_to_jiffies() and jiffies_to_msecs() dance.
>>>
>>> (That would be done in a patch of its own, of course)
>>
>> tcp_keepalive_timer() does use icsk->icsk_user_timeout directly in jiffies unit,
>> but considering keeapalive timers are rarely used, this point would have to
>> do the msecs_to_jiffies() conversion.
>
> and also if icsk->icsk_user_timeout = 0, then timeout in retransmits_timed_out()
> is in jiffies and that would need to addressed.
Absolutely, this is what I was suggesting.
Pseudo code for this part, before your changes.
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 3b3611729928f77934e0298bb248e55c7a7c5def..cae7bbc956ed51e9d381650957f54550cc0967d9 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -183,8 +183,9 @@ static bool retransmits_timed_out(struct sock *sk,
else
timeout = ((2 << linear_backoff_thresh) - 1) * rto_base +
(boundary - linear_backoff_thresh) * TCP_RTO_MAX;
+ timeout = jiffies_to_msecs(timeout)'
}
- return (tcp_time_stamp(tcp_sk(sk)) - start_ts) >= jiffies_to_msecs(timeout);
+ return (tcp_time_stamp(tcp_sk(sk)) - start_ts) >= timeout;
}
/* A write timeout has occurred. Process the after effects. */
^ permalink raw reply related
* Re: [RFC net-next 00/15] net: A socket API for LoRa
From: Andreas Färber @ 2018-07-11 2:07 UTC (permalink / raw)
To: Helmut Tschemernjak
Cc: Stefan Schmidt, netdev, linux-arm-kernel, linux-kernel,
Jian-Hong Pan, Jiri Pirko, Marcel Holtmann, David S . Miller,
Matthias Brugger, Janus Piwek, Michael Röder, Dollar Chen,
Ken Yu, Konstantin Böhm, Jan Jongboom, Jon Ortego, contact,
Ben Whitten, Brian Ray, lora, lora
In-Reply-To: <5aac9ea5-484c-1b3b-8058-50e11e8d7a9a@helios.de>
Dear Helmut,
Am 05.07.2018 um 12:43 schrieb Helmut Tschemernjak:
> I put the kernel support for the SX1276 LoRa chip in question. I don’t
> think that this kind of device should be in the Linux kernel.
Thanks for sharing your opinion.
> Here are a few facts to consider:
> - A LoRa transaction is very slow (e.g. SF7 needs about 210 ms, for
> SF12 6280 ms) for 12 bytes user data with acknowledge.
Where do you see a problem? If you look at my SX1276 patch, you will
find that it queues a work item for the transmission (asynchronously)
and has an interrupt handler to get notified of the TX interrupt. It
surely won't get quicker in userspace - and as you point out, we're not
speaking about DPDK performance here, so no need for polling.
> - There are many different implementations for the antenna switch,
> switching between receiving/sending, PA-BOOST, 433, 868/915 MHz. (I know
> SX1276 Heltec ESP32, SX1276 Murata, RFM95-(1276), SX1276 Heltec
> STM32-L4) they are all different regarding this.
Yes, and as demonstrated with this patch series, the kernel is well able
to handle this variety of interfaces. I would say better than userspace!
As noted, I've tested both 868 MHz and 433 MHz modules. My SX1276 driver
uses the Device Tree (radio-frequency) to configure the driver when this
was a property of the module. RN2483 by contrast supports two, so a
netlink interface was suggested to configure this at runtime.
> - The LoRa chip device ID is only 8-bit which is not sufficient for
> larger networks, e.g. our RadioShuttle protocol uses compressed 32-bit
> device IDs.
What does that have to do with anything? The whole reason that you're
CC'ed on this patchset is to make sure that you or someone else can
implement your custom protocol on top of the APIs being proposed. So
my-protocol-is-better-than-theirs is no argument against this project.
The way I understand LoRa and thus designed the struct sockaddr_lora is
that there is no ID involved for LoRa at all - it is essentially a
broadcast. The 8-byte (not 8-bit) EUIs only come into play for LoRaWAN
AFAIU. Am I missing anything?
LoRaWAN, RadioShuttle, RadioHead and LR Base all have their own ways of
addressing - the question here is how to model that in Linux, whether as
one PF_LORA with different protocol options and a growing union
lora_addr inside sockaddr_lora covering all of them, or a protocol
family (requiring global number allocation) for each one of them.
> - The chip can do MTU sizes up to 2048 bytes, most protocols use less
> than 250 bytes.
How would 2048 bytes work? The FIFO buffer fits a maximum of 256 bytes.
IIRC the recommended MTU is also restricted by the airtime, i.e. SF,
bandwidth, preamble and other factors...
Some UART based modules allow a maximum of 64 bytes.
> - Applications do on-the-fly channel and spreading factor switching
> which makes it more difficult for the configuration.
That's exactly the question of how to implement individual options -
Device Tree would be fixed for the system's hardware, netlink would be
user-configurable for the whole network device, whereas socket options
would be per socket/application, and ioctls might be yet another
implementation layer.
> - The chip supports Lora modulation as well as FSK, GFSK, MSK, GMSK,
> and OOK modulation, for some use cases the other modulations are of
> interest, e.g. to communicated with other devices.
Yes, therefore I raised it in the cover letter as open point 5).
> - Power saving modes, like sleep, suspend may be required for battery
> operation.
The Linux kernel can deal with that much better than userspace. There's
pm hooks that one could implement. And SX1276 returns to standby mode
whenever an operation such as TX is completed. In sleep mode not all
settings get preserved, so they would need to be saved in the private
struct and restored on resume.
> - Cad detection is very flexible and can be differently used.
>
> - LoRa preamble may be different depending on the protocol used.
>
> - The new Lora chip SX1262 / 68 (successor of the SX1276) has different
> hardware and all different registers, it is driver incompatible with the
> SX1276. It needs an entire new driver.
Not unexpected, same as SX1301 being different. As this series shows,
multiple drivers can easily be implemented as necessary.
> - The device is not multi-process capable (only a single process can
> communicate with it).
How is that different from other half-duplex network interfaces? The
driver needs to take care of appropriate locking of its operations, and
the socket subsystem should make sure packets get queued accordingly.
> - There are SX1276 LoRa modules with a build-in protocol (LoRaWAN and
> RAW) via a serial connection only, again complete different API compared
> the SX1276 chip. Software updates for this devices are difficult.
Drivers for such modules are already demonstrated in this series.
> - I am even convinced that the LoRaWAN protocol with the concentrator
> concept is not good, the peer to peer communication and a standard MQTT
> gateway (what we do) is way more flexible.
Your opinion in all honors, LoRaWAN is an open specification, whereas
yours is closed. So whatever benefits yours may have, that way you're
unlikely to make LoRaWAN go away. People are rolling out LoRaWAN
gateways, and therefore the question is not which protocol is more
flexible but rather how all of them can be enabled equally for those
that want to use them.
> For all this reasons, I feel a user level driver task implementation is
> way more flexible. I did a lot of work/enhancements on the SX1276 link
> level driver from Semtech, it is available and maintained on mbed.org
> and supports mbed-os, Arduino and is prepared for Linux.
> https://os.mbed.com/users/Helmut64/code/SX1276GenericLib/
>
> Protocols e.g. our RadioShuttle LoRa peer to peer protocol or the
> LoRaWAN protocol can run on top of the SX1276GenericLib. We may should
> focus on such a driver library getting supported for multiple OS's (Win,
> Linux, mbed, Ardino, etc.)
>
> Again I feel a Linux kernel device driver for the SX1276 chip make
> little sense for me.
Well, feel free to use spidev if you prefer. As I pointed out, the Linux
spi maintainers don't seem to share your view - instead of white-listing
chipsets for concrete kernel drivers (as you appear to suggest here)
they rather black-list chipsets to be "allowed" to use spidev if all
else fails.
Having already done part of the implementation work, for me the question
is not whether to do a kernel driver but how to do it properly, keeping
all corner cases in mind, such as non-standard protocols like yours.
It's great that you're writing an Open Source library for mbed, but its
name indicates that it offers no abstraction like proposed here.
I'll drop you from v2 then, to not bother you about this Linux kernel
implementation anymore.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* [PATCH net-next v2 0/5] virtio: support packed ring
From: Tiwei Bie @ 2018-07-11 2:27 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel, netdev, virtio-dev; +Cc: wexu
Hello everyone,
This patch set implements packed ring support in virtio driver.
Some functional tests have been done with Jason's
packed ring implementation in vhost:
https://lkml.org/lkml/2018/7/3/33
Both of ping and netperf worked as expected.
v1 -> v2:
- Use READ_ONCE() to read event off_wrap and flags together (Jason);
- Add comments related to ccw (Jason);
RFC (v6) -> v1:
- Avoid extra virtio_wmb() in virtqueue_enable_cb_delayed_packed()
when event idx is off (Jason);
- Fix bufs calculation in virtqueue_enable_cb_delayed_packed() (Jason);
- Test the state of the desc at used_idx instead of last_used_idx
in virtqueue_enable_cb_delayed_packed() (Jason);
- Save wrap counter (as part of queue state) in the return value
of virtqueue_enable_cb_prepare_packed();
- Refine the packed ring definitions in uapi;
- Rebase on the net-next tree;
RFC v5 -> RFC v6:
- Avoid tracking addr/len/flags when DMA API isn't used (MST/Jason);
- Define wrap counter as bool (Jason);
- Use ALIGN() in vring_init_packed() (Jason);
- Avoid using pointer to track `next` in detach_buf_packed() (Jason);
- Add comments for barriers (Jason);
- Don't enable RING_PACKED on ccw for now (noticed by Jason);
- Refine the memory barrier in virtqueue_poll();
- Add a missing memory barrier in virtqueue_enable_cb_delayed_packed();
- Remove the hacks in virtqueue_enable_cb_prepare_packed();
RFC v4 -> RFC v5:
- Save DMA addr, etc in desc state (Jason);
- Track used wrap counter;
RFC v3 -> RFC v4:
- Make ID allocation support out-of-order (Jason);
- Various fixes for EVENT_IDX support;
RFC v2 -> RFC v3:
- Split into small patches (Jason);
- Add helper virtqueue_use_indirect() (Jason);
- Just set id for the last descriptor of a list (Jason);
- Calculate the prev in virtqueue_add_packed() (Jason);
- Fix/improve desc suppression code (Jason/MST);
- Refine the code layout for XXX_split/packed and wrappers (MST);
- Fix the comments and API in uapi (MST);
- Remove the BUG_ON() for indirect (Jason);
- Some other refinements and bug fixes;
RFC v1 -> RFC v2:
- Add indirect descriptor support - compile test only;
- Add event suppression supprt - compile test only;
- Move vring_packed_init() out of uapi (Jason, MST);
- Merge two loops into one in virtqueue_add_packed() (Jason);
- Split vring_unmap_one() for packed ring and split ring (Jason);
- Avoid using '%' operator (Jason);
- Rename free_head -> next_avail_idx (Jason);
- Add comments for virtio_wmb() in virtqueue_add_packed() (Jason);
- Some other refinements and bug fixes;
Thanks!
Tiwei Bie (5):
virtio: add packed ring definitions
virtio_ring: support creating packed ring
virtio_ring: add packed ring support
virtio_ring: add event idx support in packed ring
virtio_ring: enable packed ring
drivers/s390/virtio/virtio_ccw.c | 14 +
drivers/virtio/virtio_ring.c | 1365 ++++++++++++++++++++++------
include/linux/virtio_ring.h | 8 +-
include/uapi/linux/virtio_config.h | 3 +
include/uapi/linux/virtio_ring.h | 43 +
5 files changed, 1157 insertions(+), 276 deletions(-)
--
2.18.0
^ permalink raw reply
* [PATCH net-next v2 1/5] virtio: add packed ring definitions
From: Tiwei Bie @ 2018-07-11 2:27 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel, netdev, virtio-dev
Cc: wexu, jfreimann, tiwei.bie
In-Reply-To: <20180711022711.7090-1-tiwei.bie@intel.com>
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
include/uapi/linux/virtio_config.h | 3 +++
include/uapi/linux/virtio_ring.h | 43 ++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 449132c76b1c..1196e1c1d4f6 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -75,6 +75,9 @@
*/
#define VIRTIO_F_IOMMU_PLATFORM 33
+/* This feature indicates support for the packed virtqueue layout. */
+#define VIRTIO_F_RING_PACKED 34
+
/*
* Does the device support Single Root I/O Virtualization?
*/
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 6d5d5faa989b..0254a2ba29cf 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -44,6 +44,10 @@
/* This means the buffer contains a list of buffer descriptors. */
#define VRING_DESC_F_INDIRECT 4
+/* Mark a descriptor as available or used. */
+#define VRING_DESC_F_AVAIL (1ul << 7)
+#define VRING_DESC_F_USED (1ul << 15)
+
/* The Host uses this in used->flags to advise the Guest: don't kick me when
* you add a buffer. It's unreliable, so it's simply an optimization. Guest
* will still kick if it's out of buffers. */
@@ -53,6 +57,17 @@
* optimization. */
#define VRING_AVAIL_F_NO_INTERRUPT 1
+/* Enable events. */
+#define VRING_EVENT_F_ENABLE 0x0
+/* Disable events. */
+#define VRING_EVENT_F_DISABLE 0x1
+/*
+ * Enable events for a specific descriptor
+ * (as specified by Descriptor Ring Change Event Offset/Wrap Counter).
+ * Only valid if VIRTIO_RING_F_EVENT_IDX has been negotiated.
+ */
+#define VRING_EVENT_F_DESC 0x2
+
/* We support indirect buffer descriptors */
#define VIRTIO_RING_F_INDIRECT_DESC 28
@@ -171,4 +186,32 @@ static inline int vring_need_event(__u16 event_idx, __u16 new_idx, __u16 old)
return (__u16)(new_idx - event_idx - 1) < (__u16)(new_idx - old);
}
+struct vring_packed_desc_event {
+ /* Descriptor Ring Change Event Offset/Wrap Counter. */
+ __virtio16 off_wrap;
+ /* Descriptor Ring Change Event Flags. */
+ __virtio16 flags;
+};
+
+struct vring_packed_desc {
+ /* Buffer Address. */
+ __virtio64 addr;
+ /* Buffer Length. */
+ __virtio32 len;
+ /* Buffer ID. */
+ __virtio16 id;
+ /* The flags depending on descriptor type. */
+ __virtio16 flags;
+};
+
+struct vring_packed {
+ unsigned int num;
+
+ struct vring_packed_desc *desc;
+
+ struct vring_packed_desc_event *driver;
+
+ struct vring_packed_desc_event *device;
+};
+
#endif /* _UAPI_LINUX_VIRTIO_RING_H */
--
2.18.0
^ permalink raw reply related
* [PATCH net-next v2 2/5] virtio_ring: support creating packed ring
From: Tiwei Bie @ 2018-07-11 2:27 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel, netdev, virtio-dev
Cc: wexu, jfreimann, tiwei.bie
In-Reply-To: <20180711022711.7090-1-tiwei.bie@intel.com>
This commit introduces the support for creating packed ring.
All split ring specific functions are added _split suffix.
Some necessary stubs for packed ring are also added.
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
drivers/virtio/virtio_ring.c | 801 +++++++++++++++++++++++------------
include/linux/virtio_ring.h | 8 +-
2 files changed, 546 insertions(+), 263 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 814b395007b2..c4f8abc7445a 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -60,11 +60,15 @@ struct vring_desc_state {
struct vring_desc *indir_desc; /* Indirect descriptor, if any. */
};
+struct vring_desc_state_packed {
+ int next; /* The next desc state. */
+};
+
struct vring_virtqueue {
struct virtqueue vq;
- /* Actual memory layout for this queue */
- struct vring vring;
+ /* Is this a packed ring? */
+ bool packed;
/* Can we use weak barriers? */
bool weak_barriers;
@@ -86,11 +90,39 @@ struct vring_virtqueue {
/* Last used index we've seen. */
u16 last_used_idx;
- /* Last written value to avail->flags */
- u16 avail_flags_shadow;
+ union {
+ /* Available for split ring */
+ struct {
+ /* Actual memory layout for this queue. */
+ struct vring vring;
- /* Last written value to avail->idx in guest byte order */
- u16 avail_idx_shadow;
+ /* Last written value to avail->flags */
+ u16 avail_flags_shadow;
+
+ /* Last written value to avail->idx in
+ * guest byte order. */
+ u16 avail_idx_shadow;
+ };
+
+ /* Available for packed ring */
+ struct {
+ /* Actual memory layout for this queue. */
+ struct vring_packed vring_packed;
+
+ /* Driver ring wrap counter. */
+ bool avail_wrap_counter;
+
+ /* Device ring wrap counter. */
+ bool used_wrap_counter;
+
+ /* Index of the next avail descriptor. */
+ u16 next_avail_idx;
+
+ /* Last written value to driver->flags in
+ * guest byte order. */
+ u16 event_flags_shadow;
+ };
+ };
/* How to notify other side. FIXME: commonalize hcalls! */
bool (*notify)(struct virtqueue *vq);
@@ -110,11 +142,24 @@ struct vring_virtqueue {
#endif
/* Per-descriptor state. */
- struct vring_desc_state desc_state[];
+ union {
+ struct vring_desc_state desc_state[1];
+ struct vring_desc_state_packed desc_state_packed[1];
+ };
};
#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
+static inline bool virtqueue_use_indirect(struct virtqueue *_vq,
+ unsigned int total_sg)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ /* If the host supports indirect descriptor tables, and we have multiple
+ * buffers, then go indirect. FIXME: tune this threshold */
+ return (vq->indirect && total_sg > 1 && vq->vq.num_free);
+}
+
/*
* Modern virtio devices have feature bits to specify whether they need a
* quirk and bypass the IOMMU. If not there, just use the DMA API.
@@ -200,8 +245,17 @@ static dma_addr_t vring_map_single(const struct vring_virtqueue *vq,
cpu_addr, size, direction);
}
-static void vring_unmap_one(const struct vring_virtqueue *vq,
- struct vring_desc *desc)
+static int vring_mapping_error(const struct vring_virtqueue *vq,
+ dma_addr_t addr)
+{
+ if (!vring_use_dma_api(vq->vq.vdev))
+ return 0;
+
+ return dma_mapping_error(vring_dma_dev(vq), addr);
+}
+
+static void vring_unmap_one_split(const struct vring_virtqueue *vq,
+ struct vring_desc *desc)
{
u16 flags;
@@ -225,17 +279,9 @@ static void vring_unmap_one(const struct vring_virtqueue *vq,
}
}
-static int vring_mapping_error(const struct vring_virtqueue *vq,
- dma_addr_t addr)
-{
- if (!vring_use_dma_api(vq->vq.vdev))
- return 0;
-
- return dma_mapping_error(vring_dma_dev(vq), addr);
-}
-
-static struct vring_desc *alloc_indirect(struct virtqueue *_vq,
- unsigned int total_sg, gfp_t gfp)
+static struct vring_desc *alloc_indirect_split(struct virtqueue *_vq,
+ unsigned int total_sg,
+ gfp_t gfp)
{
struct vring_desc *desc;
unsigned int i;
@@ -256,14 +302,14 @@ static struct vring_desc *alloc_indirect(struct virtqueue *_vq,
return desc;
}
-static inline int virtqueue_add(struct virtqueue *_vq,
- struct scatterlist *sgs[],
- unsigned int total_sg,
- unsigned int out_sgs,
- unsigned int in_sgs,
- void *data,
- void *ctx,
- gfp_t gfp)
+static inline int virtqueue_add_split(struct virtqueue *_vq,
+ struct scatterlist *sgs[],
+ unsigned int total_sg,
+ unsigned int out_sgs,
+ unsigned int in_sgs,
+ void *data,
+ void *ctx,
+ gfp_t gfp)
{
struct vring_virtqueue *vq = to_vvq(_vq);
struct scatterlist *sg;
@@ -299,10 +345,8 @@ static inline int virtqueue_add(struct virtqueue *_vq,
head = vq->free_head;
- /* If the host supports indirect descriptor tables, and we have multiple
- * buffers, then go indirect. FIXME: tune this threshold */
- if (vq->indirect && total_sg > 1 && vq->vq.num_free)
- desc = alloc_indirect(_vq, total_sg, gfp);
+ if (virtqueue_use_indirect(_vq, total_sg))
+ desc = alloc_indirect_split(_vq, total_sg, gfp);
else {
desc = NULL;
WARN_ON_ONCE(total_sg > vq->vring.num && !vq->indirect);
@@ -423,7 +467,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
for (n = 0; n < total_sg; n++) {
if (i == err_idx)
break;
- vring_unmap_one(vq, &desc[i]);
+ vring_unmap_one_split(vq, &desc[i]);
i = virtio16_to_cpu(_vq->vdev, vq->vring.desc[i].next);
}
@@ -434,6 +478,355 @@ static inline int virtqueue_add(struct virtqueue *_vq,
return -EIO;
}
+static bool virtqueue_kick_prepare_split(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ u16 new, old;
+ bool needs_kick;
+
+ START_USE(vq);
+ /* We need to expose available array entries before checking avail
+ * event. */
+ virtio_mb(vq->weak_barriers);
+
+ old = vq->avail_idx_shadow - vq->num_added;
+ new = vq->avail_idx_shadow;
+ vq->num_added = 0;
+
+#ifdef DEBUG
+ if (vq->last_add_time_valid) {
+ WARN_ON(ktime_to_ms(ktime_sub(ktime_get(),
+ vq->last_add_time)) > 100);
+ }
+ vq->last_add_time_valid = false;
+#endif
+
+ if (vq->event) {
+ needs_kick = vring_need_event(virtio16_to_cpu(_vq->vdev, vring_avail_event(&vq->vring)),
+ new, old);
+ } else {
+ needs_kick = !(vq->vring.used->flags & cpu_to_virtio16(_vq->vdev, VRING_USED_F_NO_NOTIFY));
+ }
+ END_USE(vq);
+ return needs_kick;
+}
+
+static void detach_buf_split(struct vring_virtqueue *vq, unsigned int head,
+ void **ctx)
+{
+ unsigned int i, j;
+ __virtio16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT);
+
+ /* Clear data ptr. */
+ vq->desc_state[head].data = NULL;
+
+ /* Put back on free list: unmap first-level descriptors and find end */
+ i = head;
+
+ while (vq->vring.desc[i].flags & nextflag) {
+ vring_unmap_one_split(vq, &vq->vring.desc[i]);
+ i = virtio16_to_cpu(vq->vq.vdev, vq->vring.desc[i].next);
+ vq->vq.num_free++;
+ }
+
+ vring_unmap_one_split(vq, &vq->vring.desc[i]);
+ vq->vring.desc[i].next = cpu_to_virtio16(vq->vq.vdev, vq->free_head);
+ vq->free_head = head;
+
+ /* Plus final descriptor */
+ vq->vq.num_free++;
+
+ if (vq->indirect) {
+ struct vring_desc *indir_desc = vq->desc_state[head].indir_desc;
+ u32 len;
+
+ /* Free the indirect table, if any, now that it's unmapped. */
+ if (!indir_desc)
+ return;
+
+ len = virtio32_to_cpu(vq->vq.vdev, vq->vring.desc[head].len);
+
+ BUG_ON(!(vq->vring.desc[head].flags &
+ cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_INDIRECT)));
+ BUG_ON(len == 0 || len % sizeof(struct vring_desc));
+
+ for (j = 0; j < len / sizeof(struct vring_desc); j++)
+ vring_unmap_one_split(vq, &indir_desc[j]);
+
+ kfree(indir_desc);
+ vq->desc_state[head].indir_desc = NULL;
+ } else if (ctx) {
+ *ctx = vq->desc_state[head].indir_desc;
+ }
+}
+
+static inline bool more_used_split(const struct vring_virtqueue *vq)
+{
+ return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, vq->vring.used->idx);
+}
+
+static void *virtqueue_get_buf_ctx_split(struct virtqueue *_vq,
+ unsigned int *len,
+ void **ctx)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ void *ret;
+ unsigned int i;
+ u16 last_used;
+
+ START_USE(vq);
+
+ if (unlikely(vq->broken)) {
+ END_USE(vq);
+ return NULL;
+ }
+
+ if (!more_used_split(vq)) {
+ pr_debug("No more buffers in queue\n");
+ END_USE(vq);
+ return NULL;
+ }
+
+ /* Only get used array entries after they have been exposed by host. */
+ virtio_rmb(vq->weak_barriers);
+
+ last_used = (vq->last_used_idx & (vq->vring.num - 1));
+ i = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].id);
+ *len = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].len);
+
+ if (unlikely(i >= vq->vring.num)) {
+ BAD_RING(vq, "id %u out of range\n", i);
+ return NULL;
+ }
+ if (unlikely(!vq->desc_state[i].data)) {
+ BAD_RING(vq, "id %u is not a head!\n", i);
+ return NULL;
+ }
+
+ /* detach_buf_split clears data, so grab it now. */
+ ret = vq->desc_state[i].data;
+ detach_buf_split(vq, i, ctx);
+ vq->last_used_idx++;
+ /* If we expect an interrupt for the next entry, tell host
+ * by writing event index and flush out the write before
+ * the read in the next get_buf call. */
+ if (!(vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT))
+ virtio_store_mb(vq->weak_barriers,
+ &vring_used_event(&vq->vring),
+ cpu_to_virtio16(_vq->vdev, vq->last_used_idx));
+
+#ifdef DEBUG
+ vq->last_add_time_valid = false;
+#endif
+
+ END_USE(vq);
+ return ret;
+}
+
+static void virtqueue_disable_cb_split(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ if (!(vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT)) {
+ vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
+ if (!vq->event)
+ vq->vring.avail->flags = cpu_to_virtio16(_vq->vdev, vq->avail_flags_shadow);
+ }
+}
+
+static unsigned virtqueue_enable_cb_prepare_split(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ u16 last_used_idx;
+
+ START_USE(vq);
+
+ /* We optimistically turn back on interrupts, then check if there was
+ * more to do. */
+ /* Depending on the VIRTIO_RING_F_EVENT_IDX feature, we need to
+ * either clear the flags bit or point the event index at the next
+ * entry. Always do both to keep code simple. */
+ if (vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT) {
+ vq->avail_flags_shadow &= ~VRING_AVAIL_F_NO_INTERRUPT;
+ if (!vq->event)
+ vq->vring.avail->flags = cpu_to_virtio16(_vq->vdev, vq->avail_flags_shadow);
+ }
+ vring_used_event(&vq->vring) = cpu_to_virtio16(_vq->vdev, last_used_idx = vq->last_used_idx);
+ END_USE(vq);
+ return last_used_idx;
+}
+
+static bool virtqueue_poll_split(struct virtqueue *_vq, unsigned last_used_idx)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ virtio_mb(vq->weak_barriers);
+ return (u16)last_used_idx != virtio16_to_cpu(_vq->vdev, vq->vring.used->idx);
+}
+
+static bool virtqueue_enable_cb_delayed_split(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ u16 bufs;
+
+ START_USE(vq);
+
+ /* We optimistically turn back on interrupts, then check if there was
+ * more to do. */
+ /* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to
+ * either clear the flags bit or point the event index at the next
+ * entry. Always update the event index to keep code simple. */
+ if (vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT) {
+ vq->avail_flags_shadow &= ~VRING_AVAIL_F_NO_INTERRUPT;
+ if (!vq->event)
+ vq->vring.avail->flags = cpu_to_virtio16(_vq->vdev, vq->avail_flags_shadow);
+ }
+ /* TODO: tune this threshold */
+ bufs = (u16)(vq->avail_idx_shadow - vq->last_used_idx) * 3 / 4;
+
+ virtio_store_mb(vq->weak_barriers,
+ &vring_used_event(&vq->vring),
+ cpu_to_virtio16(_vq->vdev, vq->last_used_idx + bufs));
+
+ if (unlikely((u16)(virtio16_to_cpu(_vq->vdev, vq->vring.used->idx) - vq->last_used_idx) > bufs)) {
+ END_USE(vq);
+ return false;
+ }
+
+ END_USE(vq);
+ return true;
+}
+
+static void *virtqueue_detach_unused_buf_split(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ unsigned int i;
+ void *buf;
+
+ START_USE(vq);
+
+ for (i = 0; i < vq->vring.num; i++) {
+ if (!vq->desc_state[i].data)
+ continue;
+ /* detach_buf clears data, so grab it now. */
+ buf = vq->desc_state[i].data;
+ detach_buf_split(vq, i, NULL);
+ vq->avail_idx_shadow--;
+ vq->vring.avail->idx = cpu_to_virtio16(_vq->vdev, vq->avail_idx_shadow);
+ END_USE(vq);
+ return buf;
+ }
+ /* That should have freed everything. */
+ BUG_ON(vq->vq.num_free != vq->vring.num);
+
+ END_USE(vq);
+ return NULL;
+}
+
+/*
+ * The layout for the packed ring is a continuous chunk of memory
+ * which looks like this.
+ *
+ * struct vring_packed {
+ * // The actual descriptors (16 bytes each)
+ * struct vring_packed_desc desc[num];
+ *
+ * // Padding to the next align boundary.
+ * char pad[];
+ *
+ * // Driver Event Suppression
+ * struct vring_packed_desc_event driver;
+ *
+ * // Device Event Suppression
+ * struct vring_packed_desc_event device;
+ * };
+ */
+static inline void vring_init_packed(struct vring_packed *vr, unsigned int num,
+ void *p, unsigned long align)
+{
+ vr->num = num;
+ vr->desc = p;
+ vr->driver = (void *)ALIGN(((uintptr_t)p +
+ sizeof(struct vring_packed_desc) * num), align);
+ vr->device = vr->driver + 1;
+}
+
+static inline unsigned vring_size_packed(unsigned int num, unsigned long align)
+{
+ return ((sizeof(struct vring_packed_desc) * num + align - 1)
+ & ~(align - 1)) + sizeof(struct vring_packed_desc_event) * 2;
+}
+
+static inline int virtqueue_add_packed(struct virtqueue *_vq,
+ struct scatterlist *sgs[],
+ unsigned int total_sg,
+ unsigned int out_sgs,
+ unsigned int in_sgs,
+ void *data,
+ void *ctx,
+ gfp_t gfp)
+{
+ return -EIO;
+}
+
+static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
+{
+ return false;
+}
+
+static inline bool more_used_packed(const struct vring_virtqueue *vq)
+{
+ return false;
+}
+
+static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq,
+ unsigned int *len,
+ void **ctx)
+{
+ return NULL;
+}
+
+static void virtqueue_disable_cb_packed(struct virtqueue *_vq)
+{
+}
+
+static unsigned virtqueue_enable_cb_prepare_packed(struct virtqueue *_vq)
+{
+ return 0;
+}
+
+static bool virtqueue_poll_packed(struct virtqueue *_vq, unsigned last_used_idx)
+{
+ return false;
+}
+
+static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq)
+{
+ return false;
+}
+
+static void *virtqueue_detach_unused_buf_packed(struct virtqueue *_vq)
+{
+ return NULL;
+}
+
+static inline int virtqueue_add(struct virtqueue *_vq,
+ struct scatterlist *sgs[],
+ unsigned int total_sg,
+ unsigned int out_sgs,
+ unsigned int in_sgs,
+ void *data,
+ void *ctx,
+ gfp_t gfp)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ return vq->packed ? virtqueue_add_packed(_vq, sgs, total_sg, out_sgs,
+ in_sgs, data, ctx, gfp) :
+ virtqueue_add_split(_vq, sgs, total_sg, out_sgs,
+ in_sgs, data, ctx, gfp);
+}
+
/**
* virtqueue_add_sgs - expose buffers to other end
* @vq: the struct virtqueue we're talking about.
@@ -550,34 +943,9 @@ EXPORT_SYMBOL_GPL(virtqueue_add_inbuf_ctx);
bool virtqueue_kick_prepare(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
- u16 new, old;
- bool needs_kick;
- START_USE(vq);
- /* We need to expose available array entries before checking avail
- * event. */
- virtio_mb(vq->weak_barriers);
-
- old = vq->avail_idx_shadow - vq->num_added;
- new = vq->avail_idx_shadow;
- vq->num_added = 0;
-
-#ifdef DEBUG
- if (vq->last_add_time_valid) {
- WARN_ON(ktime_to_ms(ktime_sub(ktime_get(),
- vq->last_add_time)) > 100);
- }
- vq->last_add_time_valid = false;
-#endif
-
- if (vq->event) {
- needs_kick = vring_need_event(virtio16_to_cpu(_vq->vdev, vring_avail_event(&vq->vring)),
- new, old);
- } else {
- needs_kick = !(vq->vring.used->flags & cpu_to_virtio16(_vq->vdev, VRING_USED_F_NO_NOTIFY));
- }
- END_USE(vq);
- return needs_kick;
+ return vq->packed ? virtqueue_kick_prepare_packed(_vq) :
+ virtqueue_kick_prepare_split(_vq);
}
EXPORT_SYMBOL_GPL(virtqueue_kick_prepare);
@@ -625,58 +993,9 @@ bool virtqueue_kick(struct virtqueue *vq)
}
EXPORT_SYMBOL_GPL(virtqueue_kick);
-static void detach_buf(struct vring_virtqueue *vq, unsigned int head,
- void **ctx)
-{
- unsigned int i, j;
- __virtio16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT);
-
- /* Clear data ptr. */
- vq->desc_state[head].data = NULL;
-
- /* Put back on free list: unmap first-level descriptors and find end */
- i = head;
-
- while (vq->vring.desc[i].flags & nextflag) {
- vring_unmap_one(vq, &vq->vring.desc[i]);
- i = virtio16_to_cpu(vq->vq.vdev, vq->vring.desc[i].next);
- vq->vq.num_free++;
- }
-
- vring_unmap_one(vq, &vq->vring.desc[i]);
- vq->vring.desc[i].next = cpu_to_virtio16(vq->vq.vdev, vq->free_head);
- vq->free_head = head;
-
- /* Plus final descriptor */
- vq->vq.num_free++;
-
- if (vq->indirect) {
- struct vring_desc *indir_desc = vq->desc_state[head].indir_desc;
- u32 len;
-
- /* Free the indirect table, if any, now that it's unmapped. */
- if (!indir_desc)
- return;
-
- len = virtio32_to_cpu(vq->vq.vdev, vq->vring.desc[head].len);
-
- BUG_ON(!(vq->vring.desc[head].flags &
- cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_INDIRECT)));
- BUG_ON(len == 0 || len % sizeof(struct vring_desc));
-
- for (j = 0; j < len / sizeof(struct vring_desc); j++)
- vring_unmap_one(vq, &indir_desc[j]);
-
- kfree(indir_desc);
- vq->desc_state[head].indir_desc = NULL;
- } else if (ctx) {
- *ctx = vq->desc_state[head].indir_desc;
- }
-}
-
static inline bool more_used(const struct vring_virtqueue *vq)
{
- return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, vq->vring.used->idx);
+ return vq->packed ? more_used_packed(vq) : more_used_split(vq);
}
/**
@@ -699,57 +1018,9 @@ void *virtqueue_get_buf_ctx(struct virtqueue *_vq, unsigned int *len,
void **ctx)
{
struct vring_virtqueue *vq = to_vvq(_vq);
- void *ret;
- unsigned int i;
- u16 last_used;
- START_USE(vq);
-
- if (unlikely(vq->broken)) {
- END_USE(vq);
- return NULL;
- }
-
- if (!more_used(vq)) {
- pr_debug("No more buffers in queue\n");
- END_USE(vq);
- return NULL;
- }
-
- /* Only get used array entries after they have been exposed by host. */
- virtio_rmb(vq->weak_barriers);
-
- last_used = (vq->last_used_idx & (vq->vring.num - 1));
- i = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].id);
- *len = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].len);
-
- if (unlikely(i >= vq->vring.num)) {
- BAD_RING(vq, "id %u out of range\n", i);
- return NULL;
- }
- if (unlikely(!vq->desc_state[i].data)) {
- BAD_RING(vq, "id %u is not a head!\n", i);
- return NULL;
- }
-
- /* detach_buf clears data, so grab it now. */
- ret = vq->desc_state[i].data;
- detach_buf(vq, i, ctx);
- vq->last_used_idx++;
- /* If we expect an interrupt for the next entry, tell host
- * by writing event index and flush out the write before
- * the read in the next get_buf call. */
- if (!(vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT))
- virtio_store_mb(vq->weak_barriers,
- &vring_used_event(&vq->vring),
- cpu_to_virtio16(_vq->vdev, vq->last_used_idx));
-
-#ifdef DEBUG
- vq->last_add_time_valid = false;
-#endif
-
- END_USE(vq);
- return ret;
+ return vq->packed ? virtqueue_get_buf_ctx_packed(_vq, len, ctx) :
+ virtqueue_get_buf_ctx_split(_vq, len, ctx);
}
EXPORT_SYMBOL_GPL(virtqueue_get_buf_ctx);
@@ -771,12 +1042,10 @@ void virtqueue_disable_cb(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
- if (!(vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT)) {
- vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
- if (!vq->event)
- vq->vring.avail->flags = cpu_to_virtio16(_vq->vdev, vq->avail_flags_shadow);
- }
-
+ if (vq->packed)
+ virtqueue_disable_cb_packed(_vq);
+ else
+ virtqueue_disable_cb_split(_vq);
}
EXPORT_SYMBOL_GPL(virtqueue_disable_cb);
@@ -795,23 +1064,9 @@ EXPORT_SYMBOL_GPL(virtqueue_disable_cb);
unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
- u16 last_used_idx;
- START_USE(vq);
-
- /* We optimistically turn back on interrupts, then check if there was
- * more to do. */
- /* Depending on the VIRTIO_RING_F_EVENT_IDX feature, we need to
- * either clear the flags bit or point the event index at the next
- * entry. Always do both to keep code simple. */
- if (vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT) {
- vq->avail_flags_shadow &= ~VRING_AVAIL_F_NO_INTERRUPT;
- if (!vq->event)
- vq->vring.avail->flags = cpu_to_virtio16(_vq->vdev, vq->avail_flags_shadow);
- }
- vring_used_event(&vq->vring) = cpu_to_virtio16(_vq->vdev, last_used_idx = vq->last_used_idx);
- END_USE(vq);
- return last_used_idx;
+ return vq->packed ? virtqueue_enable_cb_prepare_packed(_vq) :
+ virtqueue_enable_cb_prepare_split(_vq);
}
EXPORT_SYMBOL_GPL(virtqueue_enable_cb_prepare);
@@ -828,8 +1083,8 @@ bool virtqueue_poll(struct virtqueue *_vq, unsigned last_used_idx)
{
struct vring_virtqueue *vq = to_vvq(_vq);
- virtio_mb(vq->weak_barriers);
- return (u16)last_used_idx != virtio16_to_cpu(_vq->vdev, vq->vring.used->idx);
+ return vq->packed ? virtqueue_poll_packed(_vq, last_used_idx) :
+ virtqueue_poll_split(_vq, last_used_idx);
}
EXPORT_SYMBOL_GPL(virtqueue_poll);
@@ -867,34 +1122,9 @@ EXPORT_SYMBOL_GPL(virtqueue_enable_cb);
bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
- u16 bufs;
- START_USE(vq);
-
- /* We optimistically turn back on interrupts, then check if there was
- * more to do. */
- /* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to
- * either clear the flags bit or point the event index at the next
- * entry. Always update the event index to keep code simple. */
- if (vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT) {
- vq->avail_flags_shadow &= ~VRING_AVAIL_F_NO_INTERRUPT;
- if (!vq->event)
- vq->vring.avail->flags = cpu_to_virtio16(_vq->vdev, vq->avail_flags_shadow);
- }
- /* TODO: tune this threshold */
- bufs = (u16)(vq->avail_idx_shadow - vq->last_used_idx) * 3 / 4;
-
- virtio_store_mb(vq->weak_barriers,
- &vring_used_event(&vq->vring),
- cpu_to_virtio16(_vq->vdev, vq->last_used_idx + bufs));
-
- if (unlikely((u16)(virtio16_to_cpu(_vq->vdev, vq->vring.used->idx) - vq->last_used_idx) > bufs)) {
- END_USE(vq);
- return false;
- }
-
- END_USE(vq);
- return true;
+ return vq->packed ? virtqueue_enable_cb_delayed_packed(_vq) :
+ virtqueue_enable_cb_delayed_split(_vq);
}
EXPORT_SYMBOL_GPL(virtqueue_enable_cb_delayed);
@@ -909,27 +1139,9 @@ EXPORT_SYMBOL_GPL(virtqueue_enable_cb_delayed);
void *virtqueue_detach_unused_buf(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
- unsigned int i;
- void *buf;
- START_USE(vq);
-
- for (i = 0; i < vq->vring.num; i++) {
- if (!vq->desc_state[i].data)
- continue;
- /* detach_buf clears data, so grab it now. */
- buf = vq->desc_state[i].data;
- detach_buf(vq, i, NULL);
- vq->avail_idx_shadow--;
- vq->vring.avail->idx = cpu_to_virtio16(_vq->vdev, vq->avail_idx_shadow);
- END_USE(vq);
- return buf;
- }
- /* That should have freed everything. */
- BUG_ON(vq->vq.num_free != vq->vring.num);
-
- END_USE(vq);
- return NULL;
+ return vq->packed ? virtqueue_detach_unused_buf_packed(_vq) :
+ virtqueue_detach_unused_buf_split(_vq);
}
EXPORT_SYMBOL_GPL(virtqueue_detach_unused_buf);
@@ -954,7 +1166,8 @@ irqreturn_t vring_interrupt(int irq, void *_vq)
EXPORT_SYMBOL_GPL(vring_interrupt);
struct virtqueue *__vring_new_virtqueue(unsigned int index,
- struct vring vring,
+ union vring_union vring,
+ bool packed,
struct virtio_device *vdev,
bool weak_barriers,
bool context,
@@ -962,19 +1175,22 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
void (*callback)(struct virtqueue *),
const char *name)
{
- unsigned int i;
struct vring_virtqueue *vq;
+ unsigned int num, i;
+ size_t size;
- vq = kmalloc(sizeof(*vq) + vring.num * sizeof(struct vring_desc_state),
- GFP_KERNEL);
+ num = packed ? vring.vring_packed.num : vring.vring_split.num;
+ size = packed ? num * sizeof(struct vring_desc_state_packed) :
+ num * sizeof(struct vring_desc_state);
+
+ vq = kmalloc(sizeof(*vq) + size, GFP_KERNEL);
if (!vq)
return NULL;
- vq->vring = vring;
vq->vq.callback = callback;
vq->vq.vdev = vdev;
vq->vq.name = name;
- vq->vq.num_free = vring.num;
+ vq->vq.num_free = num;
vq->vq.index = index;
vq->we_own_ring = false;
vq->queue_dma_addr = 0;
@@ -983,9 +1199,8 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
vq->weak_barriers = weak_barriers;
vq->broken = false;
vq->last_used_idx = 0;
- vq->avail_flags_shadow = 0;
- vq->avail_idx_shadow = 0;
vq->num_added = 0;
+ vq->packed = packed;
list_add_tail(&vq->vq.list, &vdev->vqs);
#ifdef DEBUG
vq->in_use = false;
@@ -996,19 +1211,48 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
!context;
vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
+ if (vq->packed) {
+ vq->vring_packed = vring.vring_packed;
+ vq->next_avail_idx = 0;
+ vq->avail_wrap_counter = 1;
+ vq->used_wrap_counter = 1;
+ vq->event_flags_shadow = 0;
+
+ memset(vq->desc_state_packed, 0,
+ num * sizeof(struct vring_desc_state_packed));
+
+ /* Put everything in free lists. */
+ vq->free_head = 0;
+ for (i = 0; i < num-1; i++)
+ vq->desc_state_packed[i].next = i + 1;
+ } else {
+ vq->vring = vring.vring_split;
+ vq->avail_flags_shadow = 0;
+ vq->avail_idx_shadow = 0;
+
+ /* Put everything in free lists. */
+ vq->free_head = 0;
+ for (i = 0; i < num-1; i++)
+ vq->vring.desc[i].next = cpu_to_virtio16(vdev, i + 1);
+
+ memset(vq->desc_state, 0,
+ num * sizeof(struct vring_desc_state));
+ }
+
/* No callback? Tell other side not to bother us. */
if (!callback) {
- vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
- if (!vq->event)
- vq->vring.avail->flags = cpu_to_virtio16(vdev, vq->avail_flags_shadow);
+ if (packed) {
+ vq->event_flags_shadow = VRING_EVENT_F_DISABLE;
+ vq->vring_packed.driver->flags = cpu_to_virtio16(vdev,
+ vq->event_flags_shadow);
+ } else {
+ vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
+ if (!vq->event)
+ vq->vring.avail->flags = cpu_to_virtio16(vdev,
+ vq->avail_flags_shadow);
+ }
}
- /* Put everything in free lists. */
- vq->free_head = 0;
- for (i = 0; i < vring.num-1; i++)
- vq->vring.desc[i].next = cpu_to_virtio16(vdev, i + 1);
- memset(vq->desc_state, 0, vring.num * sizeof(struct vring_desc_state));
-
return &vq->vq;
}
EXPORT_SYMBOL_GPL(__vring_new_virtqueue);
@@ -1055,6 +1299,12 @@ static void vring_free_queue(struct virtio_device *vdev, size_t size,
}
}
+static inline int
+__vring_size(unsigned int num, unsigned long align, bool packed)
+{
+ return packed ? vring_size_packed(num, align) : vring_size(num, align);
+}
+
struct virtqueue *vring_create_virtqueue(
unsigned int index,
unsigned int num,
@@ -1071,7 +1321,8 @@ struct virtqueue *vring_create_virtqueue(
void *queue = NULL;
dma_addr_t dma_addr;
size_t queue_size_in_bytes;
- struct vring vring;
+ union vring_union vring;
+ bool packed;
/* We assume num is a power of 2. */
if (num & (num - 1)) {
@@ -1079,9 +1330,13 @@ struct virtqueue *vring_create_virtqueue(
return NULL;
}
+ packed = virtio_has_feature(vdev, VIRTIO_F_RING_PACKED);
+
/* TODO: allocate each queue chunk individually */
- for (; num && vring_size(num, vring_align) > PAGE_SIZE; num /= 2) {
- queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+ for (; num && __vring_size(num, vring_align, packed) > PAGE_SIZE;
+ num /= 2) {
+ queue = vring_alloc_queue(vdev, __vring_size(num, vring_align,
+ packed),
&dma_addr,
GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO);
if (queue)
@@ -1093,17 +1348,21 @@ struct virtqueue *vring_create_virtqueue(
if (!queue) {
/* Try to get a single page. You are my only hope! */
- queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+ queue = vring_alloc_queue(vdev, __vring_size(num, vring_align,
+ packed),
&dma_addr, GFP_KERNEL|__GFP_ZERO);
}
if (!queue)
return NULL;
- queue_size_in_bytes = vring_size(num, vring_align);
- vring_init(&vring, num, queue, vring_align);
+ queue_size_in_bytes = __vring_size(num, vring_align, packed);
+ if (packed)
+ vring_init_packed(&vring.vring_packed, num, queue, vring_align);
+ else
+ vring_init(&vring.vring_split, num, queue, vring_align);
- vq = __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
- notify, callback, name);
+ vq = __vring_new_virtqueue(index, vring, packed, vdev, weak_barriers,
+ context, notify, callback, name);
if (!vq) {
vring_free_queue(vdev, queue_size_in_bytes, queue,
dma_addr);
@@ -1129,10 +1388,17 @@ struct virtqueue *vring_new_virtqueue(unsigned int index,
void (*callback)(struct virtqueue *vq),
const char *name)
{
- struct vring vring;
- vring_init(&vring, num, pages, vring_align);
- return __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
- notify, callback, name);
+ union vring_union vring;
+ bool packed;
+
+ packed = virtio_has_feature(vdev, VIRTIO_F_RING_PACKED);
+ if (packed)
+ vring_init_packed(&vring.vring_packed, num, pages, vring_align);
+ else
+ vring_init(&vring.vring_split, num, pages, vring_align);
+
+ return __vring_new_virtqueue(index, vring, packed, vdev, weak_barriers,
+ context, notify, callback, name);
}
EXPORT_SYMBOL_GPL(vring_new_virtqueue);
@@ -1142,7 +1408,9 @@ void vring_del_virtqueue(struct virtqueue *_vq)
if (vq->we_own_ring) {
vring_free_queue(vq->vq.vdev, vq->queue_size_in_bytes,
- vq->vring.desc, vq->queue_dma_addr);
+ vq->packed ? (void *)vq->vring_packed.desc :
+ (void *)vq->vring.desc,
+ vq->queue_dma_addr);
}
list_del(&_vq->list);
kfree(vq);
@@ -1184,7 +1452,7 @@ unsigned int virtqueue_get_vring_size(struct virtqueue *_vq)
struct vring_virtqueue *vq = to_vvq(_vq);
- return vq->vring.num;
+ return vq->packed ? vq->vring_packed.num : vq->vring.num;
}
EXPORT_SYMBOL_GPL(virtqueue_get_vring_size);
@@ -1227,6 +1495,10 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
BUG_ON(!vq->we_own_ring);
+ if (vq->packed)
+ return vq->queue_dma_addr + ((char *)vq->vring_packed.driver -
+ (char *)vq->vring_packed.desc);
+
return vq->queue_dma_addr +
((char *)vq->vring.avail - (char *)vq->vring.desc);
}
@@ -1238,11 +1510,16 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
BUG_ON(!vq->we_own_ring);
+ if (vq->packed)
+ return vq->queue_dma_addr + ((char *)vq->vring_packed.device -
+ (char *)vq->vring_packed.desc);
+
return vq->queue_dma_addr +
((char *)vq->vring.used - (char *)vq->vring.desc);
}
EXPORT_SYMBOL_GPL(virtqueue_get_used_addr);
+/* Only available for split ring */
const struct vring *virtqueue_get_vring(struct virtqueue *vq)
{
return &to_vvq(vq)->vring;
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index fab02133a919..992142b35f55 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -60,6 +60,11 @@ static inline void virtio_store_mb(bool weak_barriers,
struct virtio_device;
struct virtqueue;
+union vring_union {
+ struct vring vring_split;
+ struct vring_packed vring_packed;
+};
+
/*
* Creates a virtqueue and allocates the descriptor ring. If
* may_reduce_num is set, then this may allocate a smaller ring than
@@ -79,7 +84,8 @@ struct virtqueue *vring_create_virtqueue(unsigned int index,
/* Creates a virtqueue with a custom layout. */
struct virtqueue *__vring_new_virtqueue(unsigned int index,
- struct vring vring,
+ union vring_union vring,
+ bool packed,
struct virtio_device *vdev,
bool weak_barriers,
bool ctx,
--
2.18.0
^ permalink raw reply related
* [PATCH net-next v2 3/5] virtio_ring: add packed ring support
From: Tiwei Bie @ 2018-07-11 2:27 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel, netdev, virtio-dev
Cc: wexu, jfreimann, tiwei.bie
In-Reply-To: <20180711022711.7090-1-tiwei.bie@intel.com>
This commit introduces the support (without EVENT_IDX) for
packed ring.
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
drivers/virtio/virtio_ring.c | 495 ++++++++++++++++++++++++++++++++++-
1 file changed, 487 insertions(+), 8 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index c4f8abc7445a..f317b485ba54 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -55,12 +55,21 @@
#define END_USE(vq)
#endif
+#define _VRING_DESC_F_AVAIL(b) ((__u16)(b) << 7)
+#define _VRING_DESC_F_USED(b) ((__u16)(b) << 15)
+
struct vring_desc_state {
void *data; /* Data for callback. */
struct vring_desc *indir_desc; /* Indirect descriptor, if any. */
};
struct vring_desc_state_packed {
+ void *data; /* Data for callback. */
+ struct vring_packed_desc *indir_desc; /* Indirect descriptor, if any. */
+ int num; /* Descriptor list length. */
+ dma_addr_t addr; /* Buffer DMA addr. */
+ u32 len; /* Buffer length. */
+ u16 flags; /* Descriptor flags. */
int next; /* The next desc state. */
};
@@ -660,7 +669,6 @@ static bool virtqueue_poll_split(struct virtqueue *_vq, unsigned last_used_idx)
{
struct vring_virtqueue *vq = to_vvq(_vq);
- virtio_mb(vq->weak_barriers);
return (u16)last_used_idx != virtio16_to_cpu(_vq->vdev, vq->vring.used->idx);
}
@@ -757,6 +765,72 @@ static inline unsigned vring_size_packed(unsigned int num, unsigned long align)
& ~(align - 1)) + sizeof(struct vring_packed_desc_event) * 2;
}
+static void vring_unmap_state_packed(const struct vring_virtqueue *vq,
+ struct vring_desc_state_packed *state)
+{
+ u16 flags;
+
+ if (!vring_use_dma_api(vq->vq.vdev))
+ return;
+
+ flags = state->flags;
+
+ if (flags & VRING_DESC_F_INDIRECT) {
+ dma_unmap_single(vring_dma_dev(vq),
+ state->addr, state->len,
+ (flags & VRING_DESC_F_WRITE) ?
+ DMA_FROM_DEVICE : DMA_TO_DEVICE);
+ } else {
+ dma_unmap_page(vring_dma_dev(vq),
+ state->addr, state->len,
+ (flags & VRING_DESC_F_WRITE) ?
+ DMA_FROM_DEVICE : DMA_TO_DEVICE);
+ }
+}
+
+static void vring_unmap_desc_packed(const struct vring_virtqueue *vq,
+ struct vring_packed_desc *desc)
+{
+ u16 flags;
+
+ if (!vring_use_dma_api(vq->vq.vdev))
+ return;
+
+ flags = virtio16_to_cpu(vq->vq.vdev, desc->flags);
+
+ if (flags & VRING_DESC_F_INDIRECT) {
+ dma_unmap_single(vring_dma_dev(vq),
+ virtio64_to_cpu(vq->vq.vdev, desc->addr),
+ virtio32_to_cpu(vq->vq.vdev, desc->len),
+ (flags & VRING_DESC_F_WRITE) ?
+ DMA_FROM_DEVICE : DMA_TO_DEVICE);
+ } else {
+ dma_unmap_page(vring_dma_dev(vq),
+ virtio64_to_cpu(vq->vq.vdev, desc->addr),
+ virtio32_to_cpu(vq->vq.vdev, desc->len),
+ (flags & VRING_DESC_F_WRITE) ?
+ DMA_FROM_DEVICE : DMA_TO_DEVICE);
+ }
+}
+
+static struct vring_packed_desc *alloc_indirect_packed(struct virtqueue *_vq,
+ unsigned int total_sg,
+ gfp_t gfp)
+{
+ struct vring_packed_desc *desc;
+
+ /*
+ * We require lowmem mappings for the descriptors because
+ * otherwise virt_to_phys will give us bogus addresses in the
+ * virtqueue.
+ */
+ gfp &= ~__GFP_HIGHMEM;
+
+ desc = kmalloc(total_sg * sizeof(struct vring_packed_desc), gfp);
+
+ return desc;
+}
+
static inline int virtqueue_add_packed(struct virtqueue *_vq,
struct scatterlist *sgs[],
unsigned int total_sg,
@@ -766,47 +840,449 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
void *ctx,
gfp_t gfp)
{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ struct vring_packed_desc *desc;
+ struct scatterlist *sg;
+ unsigned int i, n, descs_used, uninitialized_var(prev), err_idx;
+ __virtio16 uninitialized_var(head_flags), flags;
+ u16 head, avail_wrap_counter, id, curr;
+ bool indirect;
+
+ START_USE(vq);
+
+ BUG_ON(data == NULL);
+ BUG_ON(ctx && vq->indirect);
+
+ if (unlikely(vq->broken)) {
+ END_USE(vq);
+ return -EIO;
+ }
+
+#ifdef DEBUG
+ {
+ ktime_t now = ktime_get();
+
+ /* No kick or get, with .1 second between? Warn. */
+ if (vq->last_add_time_valid)
+ WARN_ON(ktime_to_ms(ktime_sub(now, vq->last_add_time))
+ > 100);
+ vq->last_add_time = now;
+ vq->last_add_time_valid = true;
+ }
+#endif
+
+ BUG_ON(total_sg == 0);
+
+ head = vq->next_avail_idx;
+ avail_wrap_counter = vq->avail_wrap_counter;
+
+ if (virtqueue_use_indirect(_vq, total_sg))
+ desc = alloc_indirect_packed(_vq, total_sg, gfp);
+ else {
+ desc = NULL;
+ WARN_ON_ONCE(total_sg > vq->vring_packed.num && !vq->indirect);
+ }
+
+ if (desc) {
+ /* Use a single buffer which doesn't continue */
+ indirect = true;
+ /* Set up rest to use this indirect table. */
+ i = 0;
+ descs_used = 1;
+ } else {
+ indirect = false;
+ desc = vq->vring_packed.desc;
+ i = head;
+ descs_used = total_sg;
+ }
+
+ if (vq->vq.num_free < descs_used) {
+ pr_debug("Can't add buf len %i - avail = %i\n",
+ descs_used, vq->vq.num_free);
+ /* FIXME: for historical reasons, we force a notify here if
+ * there are outgoing parts to the buffer. Presumably the
+ * host should service the ring ASAP. */
+ if (out_sgs)
+ vq->notify(&vq->vq);
+ if (indirect)
+ kfree(desc);
+ END_USE(vq);
+ return -ENOSPC;
+ }
+
+ id = vq->free_head;
+ BUG_ON(id == vq->vring_packed.num);
+
+ curr = id;
+ for (n = 0; n < out_sgs + in_sgs; n++) {
+ for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+ dma_addr_t addr = vring_map_one_sg(vq, sg, n < out_sgs ?
+ DMA_TO_DEVICE : DMA_FROM_DEVICE);
+ if (vring_mapping_error(vq, addr))
+ goto unmap_release;
+
+ flags = cpu_to_virtio16(_vq->vdev, VRING_DESC_F_NEXT |
+ (n < out_sgs ? 0 : VRING_DESC_F_WRITE) |
+ _VRING_DESC_F_AVAIL(vq->avail_wrap_counter) |
+ _VRING_DESC_F_USED(!vq->avail_wrap_counter));
+ if (!indirect && i == head)
+ head_flags = flags;
+ else
+ desc[i].flags = flags;
+
+ desc[i].addr = cpu_to_virtio64(_vq->vdev, addr);
+ desc[i].len = cpu_to_virtio32(_vq->vdev, sg->length);
+ i++;
+ if (!indirect) {
+ if (vring_use_dma_api(_vq->vdev)) {
+ vq->desc_state_packed[curr].addr = addr;
+ vq->desc_state_packed[curr].len =
+ sg->length;
+ vq->desc_state_packed[curr].flags =
+ virtio16_to_cpu(_vq->vdev,
+ flags);
+ }
+ curr = vq->desc_state_packed[curr].next;
+
+ if (i >= vq->vring_packed.num) {
+ i = 0;
+ vq->avail_wrap_counter ^= 1;
+ }
+ }
+ }
+ }
+
+ prev = (i > 0 ? i : vq->vring_packed.num) - 1;
+ desc[prev].id = cpu_to_virtio16(_vq->vdev, id);
+
+ /* Last one doesn't continue. */
+ if (total_sg == 1)
+ head_flags &= cpu_to_virtio16(_vq->vdev, ~VRING_DESC_F_NEXT);
+ else
+ desc[prev].flags &= cpu_to_virtio16(_vq->vdev,
+ ~VRING_DESC_F_NEXT);
+
+ if (indirect) {
+ /* Now that the indirect table is filled in, map it. */
+ dma_addr_t addr = vring_map_single(
+ vq, desc, total_sg * sizeof(struct vring_packed_desc),
+ DMA_TO_DEVICE);
+ if (vring_mapping_error(vq, addr))
+ goto unmap_release;
+
+ head_flags = cpu_to_virtio16(_vq->vdev, VRING_DESC_F_INDIRECT |
+ _VRING_DESC_F_AVAIL(avail_wrap_counter) |
+ _VRING_DESC_F_USED(!avail_wrap_counter));
+ vq->vring_packed.desc[head].addr = cpu_to_virtio64(_vq->vdev,
+ addr);
+ vq->vring_packed.desc[head].len = cpu_to_virtio32(_vq->vdev,
+ total_sg * sizeof(struct vring_packed_desc));
+ vq->vring_packed.desc[head].id = cpu_to_virtio16(_vq->vdev, id);
+
+ if (vring_use_dma_api(_vq->vdev)) {
+ vq->desc_state_packed[id].addr = addr;
+ vq->desc_state_packed[id].len = total_sg *
+ sizeof(struct vring_packed_desc);
+ vq->desc_state_packed[id].flags =
+ virtio16_to_cpu(_vq->vdev, head_flags);
+ }
+ }
+
+ /* We're using some buffers from the free list. */
+ vq->vq.num_free -= descs_used;
+
+ /* Update free pointer */
+ if (indirect) {
+ n = head + 1;
+ if (n >= vq->vring_packed.num) {
+ n = 0;
+ vq->avail_wrap_counter ^= 1;
+ }
+ vq->next_avail_idx = n;
+ vq->free_head = vq->desc_state_packed[id].next;
+ } else {
+ vq->next_avail_idx = i;
+ vq->free_head = curr;
+ }
+
+ /* Store token and indirect buffer state. */
+ vq->desc_state_packed[id].num = descs_used;
+ vq->desc_state_packed[id].data = data;
+ if (indirect)
+ vq->desc_state_packed[id].indir_desc = desc;
+ else
+ vq->desc_state_packed[id].indir_desc = ctx;
+
+ /* A driver MUST NOT make the first descriptor in the list
+ * available before all subsequent descriptors comprising
+ * the list are made available. */
+ virtio_wmb(vq->weak_barriers);
+ vq->vring_packed.desc[head].flags = head_flags;
+ vq->num_added += descs_used;
+
+ pr_debug("Added buffer head %i to %p\n", head, vq);
+ END_USE(vq);
+
+ return 0;
+
+unmap_release:
+ err_idx = i;
+ i = head;
+
+ for (n = 0; n < total_sg; n++) {
+ if (i == err_idx)
+ break;
+ vring_unmap_desc_packed(vq, &desc[i]);
+ i++;
+ if (!indirect && i >= vq->vring_packed.num)
+ i = 0;
+ }
+
+ vq->avail_wrap_counter = avail_wrap_counter;
+
+ if (indirect)
+ kfree(desc);
+
+ END_USE(vq);
return -EIO;
}
static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
{
- return false;
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ u16 flags;
+ bool needs_kick;
+ u32 snapshot;
+
+ START_USE(vq);
+ /* We need to expose the new flags value before checking notification
+ * suppressions. */
+ virtio_mb(vq->weak_barriers);
+
+ snapshot = READ_ONCE(*(u32 *)vq->vring_packed.device);
+ flags = virtio16_to_cpu(_vq->vdev, (__virtio16)(snapshot >> 16)) & 0x3;
+
+#ifdef DEBUG
+ if (vq->last_add_time_valid) {
+ WARN_ON(ktime_to_ms(ktime_sub(ktime_get(),
+ vq->last_add_time)) > 100);
+ }
+ vq->last_add_time_valid = false;
+#endif
+
+ needs_kick = (flags != VRING_EVENT_F_DISABLE);
+ END_USE(vq);
+ return needs_kick;
+}
+
+static void detach_buf_packed(struct vring_virtqueue *vq,
+ unsigned int id, void **ctx)
+{
+ struct vring_desc_state_packed *state = NULL;
+ struct vring_packed_desc *desc;
+ unsigned int curr, i;
+
+ /* Clear data ptr. */
+ vq->desc_state_packed[id].data = NULL;
+
+ curr = id;
+ for (i = 0; i < vq->desc_state_packed[id].num; i++) {
+ state = &vq->desc_state_packed[curr];
+ vring_unmap_state_packed(vq, state);
+ curr = state->next;
+ }
+
+ BUG_ON(state == NULL);
+ vq->vq.num_free += vq->desc_state_packed[id].num;
+ state->next = vq->free_head;
+ vq->free_head = id;
+
+ if (vq->indirect) {
+ u32 len;
+
+ /* Free the indirect table, if any, now that it's unmapped. */
+ desc = vq->desc_state_packed[id].indir_desc;
+ if (!desc)
+ return;
+
+ if (vring_use_dma_api(vq->vq.vdev)) {
+ len = vq->desc_state_packed[id].len;
+ for (i = 0; i < len / sizeof(struct vring_packed_desc);
+ i++)
+ vring_unmap_desc_packed(vq, &desc[i]);
+ }
+ kfree(desc);
+ vq->desc_state_packed[id].indir_desc = NULL;
+ } else if (ctx) {
+ *ctx = vq->desc_state_packed[id].indir_desc;
+ }
+}
+
+static inline bool is_used_desc_packed(const struct vring_virtqueue *vq,
+ u16 idx, bool used_wrap_counter)
+{
+ u16 flags;
+ bool avail, used;
+
+ flags = virtio16_to_cpu(vq->vq.vdev,
+ vq->vring_packed.desc[idx].flags);
+ avail = !!(flags & VRING_DESC_F_AVAIL);
+ used = !!(flags & VRING_DESC_F_USED);
+
+ return avail == used && used == used_wrap_counter;
}
static inline bool more_used_packed(const struct vring_virtqueue *vq)
{
- return false;
+ return is_used_desc_packed(vq, vq->last_used_idx,
+ vq->used_wrap_counter);
}
static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq,
unsigned int *len,
void **ctx)
{
- return NULL;
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ u16 last_used, id;
+ void *ret;
+
+ START_USE(vq);
+
+ if (unlikely(vq->broken)) {
+ END_USE(vq);
+ return NULL;
+ }
+
+ if (!more_used_packed(vq)) {
+ pr_debug("No more buffers in queue\n");
+ END_USE(vq);
+ return NULL;
+ }
+
+ /* Only get used elements after they have been exposed by host. */
+ virtio_rmb(vq->weak_barriers);
+
+ last_used = vq->last_used_idx;
+ id = virtio16_to_cpu(_vq->vdev, vq->vring_packed.desc[last_used].id);
+ *len = virtio32_to_cpu(_vq->vdev, vq->vring_packed.desc[last_used].len);
+
+ if (unlikely(id >= vq->vring_packed.num)) {
+ BAD_RING(vq, "id %u out of range\n", id);
+ return NULL;
+ }
+ if (unlikely(!vq->desc_state_packed[id].data)) {
+ BAD_RING(vq, "id %u is not a head!\n", id);
+ return NULL;
+ }
+
+ vq->last_used_idx += vq->desc_state_packed[id].num;
+ if (vq->last_used_idx >= vq->vring_packed.num) {
+ vq->last_used_idx -= vq->vring_packed.num;
+ vq->used_wrap_counter ^= 1;
+ }
+
+ /* detach_buf_packed clears data, so grab it now. */
+ ret = vq->desc_state_packed[id].data;
+ detach_buf_packed(vq, id, ctx);
+
+#ifdef DEBUG
+ vq->last_add_time_valid = false;
+#endif
+
+ END_USE(vq);
+ return ret;
}
static void virtqueue_disable_cb_packed(struct virtqueue *_vq)
{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ if (vq->event_flags_shadow != VRING_EVENT_F_DISABLE) {
+ vq->event_flags_shadow = VRING_EVENT_F_DISABLE;
+ vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
+ vq->event_flags_shadow);
+ }
}
static unsigned virtqueue_enable_cb_prepare_packed(struct virtqueue *_vq)
{
- return 0;
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ START_USE(vq);
+
+ /* We optimistically turn back on interrupts, then check if there was
+ * more to do. */
+
+ if (vq->event_flags_shadow == VRING_EVENT_F_DISABLE) {
+ vq->event_flags_shadow = VRING_EVENT_F_ENABLE;
+ vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
+ vq->event_flags_shadow);
+ }
+
+ END_USE(vq);
+ return vq->last_used_idx | ((u16)vq->used_wrap_counter << 15);
}
-static bool virtqueue_poll_packed(struct virtqueue *_vq, unsigned last_used_idx)
+static bool virtqueue_poll_packed(struct virtqueue *_vq, unsigned off_wrap)
{
- return false;
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ bool wrap_counter;
+ u16 used_idx;
+
+ wrap_counter = off_wrap >> 15;
+ used_idx = off_wrap & ~(1 << 15);
+
+ return is_used_desc_packed(vq, used_idx, wrap_counter);
}
static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq)
{
- return false;
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ START_USE(vq);
+
+ /* We optimistically turn back on interrupts, then check if there was
+ * more to do. */
+
+ if (vq->event_flags_shadow == VRING_EVENT_F_DISABLE) {
+ vq->event_flags_shadow = VRING_EVENT_F_ENABLE;
+ vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
+ vq->event_flags_shadow);
+ /* We need to enable interrupts first before re-checking
+ * for more used buffers. */
+ virtio_mb(vq->weak_barriers);
+ }
+
+ if (more_used_packed(vq)) {
+ END_USE(vq);
+ return false;
+ }
+
+ END_USE(vq);
+ return true;
}
static void *virtqueue_detach_unused_buf_packed(struct virtqueue *_vq)
{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ unsigned int i;
+ void *buf;
+
+ START_USE(vq);
+
+ for (i = 0; i < vq->vring_packed.num; i++) {
+ if (!vq->desc_state_packed[i].data)
+ continue;
+ /* detach_buf clears data, so grab it now. */
+ buf = vq->desc_state_packed[i].data;
+ detach_buf_packed(vq, i, NULL);
+ END_USE(vq);
+ return buf;
+ }
+ /* That should have freed everything. */
+ BUG_ON(vq->vq.num_free != vq->vring_packed.num);
+
+ END_USE(vq);
return NULL;
}
@@ -1083,6 +1559,9 @@ bool virtqueue_poll(struct virtqueue *_vq, unsigned last_used_idx)
{
struct vring_virtqueue *vq = to_vvq(_vq);
+ /* We need to enable interrupts first before re-checking
+ * for more used buffers. */
+ virtio_mb(vq->weak_barriers);
return vq->packed ? virtqueue_poll_packed(_vq, last_used_idx) :
virtqueue_poll_split(_vq, last_used_idx);
}
--
2.18.0
^ permalink raw reply related
* [PATCH net-next v2 4/5] virtio_ring: add event idx support in packed ring
From: Tiwei Bie @ 2018-07-11 2:27 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel, netdev, virtio-dev
Cc: wexu, jfreimann, tiwei.bie
In-Reply-To: <20180711022711.7090-1-tiwei.bie@intel.com>
This commit introduces the EVENT_IDX support in packed ring.
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
drivers/virtio/virtio_ring.c | 73 ++++++++++++++++++++++++++++++++----
1 file changed, 65 insertions(+), 8 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index f317b485ba54..f79a1e17f7d1 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -1050,7 +1050,7 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
- u16 flags;
+ u16 new, old, off_wrap, flags, wrap_counter, event_idx;
bool needs_kick;
u32 snapshot;
@@ -1059,9 +1059,19 @@ static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
* suppressions. */
virtio_mb(vq->weak_barriers);
+ old = vq->next_avail_idx - vq->num_added;
+ new = vq->next_avail_idx;
+ vq->num_added = 0;
+
snapshot = READ_ONCE(*(u32 *)vq->vring_packed.device);
+ off_wrap = virtio16_to_cpu(_vq->vdev, (__virtio16)(snapshot & 0xffff));
flags = virtio16_to_cpu(_vq->vdev, (__virtio16)(snapshot >> 16)) & 0x3;
+ wrap_counter = off_wrap >> 15;
+ event_idx = off_wrap & ~(1 << 15);
+ if (wrap_counter != vq->avail_wrap_counter)
+ event_idx -= vq->vring_packed.num;
+
#ifdef DEBUG
if (vq->last_add_time_valid) {
WARN_ON(ktime_to_ms(ktime_sub(ktime_get(),
@@ -1070,7 +1080,10 @@ static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
vq->last_add_time_valid = false;
#endif
- needs_kick = (flags != VRING_EVENT_F_DISABLE);
+ if (flags == VRING_EVENT_F_DESC)
+ needs_kick = vring_need_event(event_idx, new, old);
+ else
+ needs_kick = (flags != VRING_EVENT_F_DISABLE);
END_USE(vq);
return needs_kick;
}
@@ -1185,6 +1198,15 @@ static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq,
ret = vq->desc_state_packed[id].data;
detach_buf_packed(vq, id, ctx);
+ /* If we expect an interrupt for the next entry, tell host
+ * by writing event index and flush out the write before
+ * the read in the next get_buf call. */
+ if (vq->event_flags_shadow == VRING_EVENT_F_DESC)
+ virtio_store_mb(vq->weak_barriers,
+ &vq->vring_packed.driver->off_wrap,
+ cpu_to_virtio16(_vq->vdev, vq->last_used_idx |
+ ((u16)vq->used_wrap_counter << 15)));
+
#ifdef DEBUG
vq->last_add_time_valid = false;
#endif
@@ -1213,8 +1235,18 @@ static unsigned virtqueue_enable_cb_prepare_packed(struct virtqueue *_vq)
/* We optimistically turn back on interrupts, then check if there was
* more to do. */
+ if (vq->event) {
+ vq->vring_packed.driver->off_wrap = cpu_to_virtio16(_vq->vdev,
+ vq->last_used_idx |
+ ((u16)vq->used_wrap_counter << 15));
+ /* We need to update event offset and event wrap
+ * counter first before updating event flags. */
+ virtio_wmb(vq->weak_barriers);
+ }
+
if (vq->event_flags_shadow == VRING_EVENT_F_DISABLE) {
- vq->event_flags_shadow = VRING_EVENT_F_ENABLE;
+ vq->event_flags_shadow = vq->event ? VRING_EVENT_F_DESC :
+ VRING_EVENT_F_ENABLE;
vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
vq->event_flags_shadow);
}
@@ -1238,22 +1270,47 @@ static bool virtqueue_poll_packed(struct virtqueue *_vq, unsigned off_wrap)
static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
+ u16 bufs, used_idx, wrap_counter;
START_USE(vq);
/* We optimistically turn back on interrupts, then check if there was
* more to do. */
+ if (vq->event) {
+ /* TODO: tune this threshold */
+ bufs = (vq->vring_packed.num - _vq->num_free) * 3 / 4;
+ wrap_counter = vq->used_wrap_counter;
+
+ used_idx = vq->last_used_idx + bufs;
+ if (used_idx >= vq->vring_packed.num) {
+ used_idx -= vq->vring_packed.num;
+ wrap_counter ^= 1;
+ }
+
+ vq->vring_packed.driver->off_wrap = cpu_to_virtio16(_vq->vdev,
+ used_idx | (wrap_counter << 15));
+
+ /* We need to update event offset and event wrap
+ * counter first before updating event flags. */
+ virtio_wmb(vq->weak_barriers);
+ } else {
+ used_idx = vq->last_used_idx;
+ wrap_counter = vq->used_wrap_counter;
+ }
+
if (vq->event_flags_shadow == VRING_EVENT_F_DISABLE) {
- vq->event_flags_shadow = VRING_EVENT_F_ENABLE;
+ vq->event_flags_shadow = vq->event ? VRING_EVENT_F_DESC :
+ VRING_EVENT_F_ENABLE;
vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
vq->event_flags_shadow);
- /* We need to enable interrupts first before re-checking
- * for more used buffers. */
- virtio_mb(vq->weak_barriers);
}
- if (more_used_packed(vq)) {
+ /* We need to update event suppression structure first
+ * before re-checking for more used buffers. */
+ virtio_mb(vq->weak_barriers);
+
+ if (is_used_desc_packed(vq, used_idx, wrap_counter)) {
END_USE(vq);
return false;
}
--
2.18.0
^ permalink raw reply related
* [PATCH net-next v2 5/5] virtio_ring: enable packed ring
From: Tiwei Bie @ 2018-07-11 2:27 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel, netdev, virtio-dev
Cc: wexu, jfreimann, tiwei.bie
In-Reply-To: <20180711022711.7090-1-tiwei.bie@intel.com>
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
drivers/s390/virtio/virtio_ccw.c | 14 ++++++++++++++
drivers/virtio/virtio_ring.c | 2 ++
2 files changed, 16 insertions(+)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 8f5c1d7f751a..8654f3a94635 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -765,6 +765,17 @@ static u64 virtio_ccw_get_features(struct virtio_device *vdev)
return rc;
}
+static void ccw_transport_features(struct virtio_device *vdev)
+{
+ /*
+ * Packed ring isn't enabled on virtio_ccw for now,
+ * because virtio_ccw uses some legacy accessors,
+ * e.g. virtqueue_get_avail() and virtqueue_get_used()
+ * which aren't available in packed ring currently.
+ */
+ __virtio_clear_bit(vdev, VIRTIO_F_RING_PACKED);
+}
+
static int virtio_ccw_finalize_features(struct virtio_device *vdev)
{
struct virtio_ccw_device *vcdev = to_vc_device(vdev);
@@ -791,6 +802,9 @@ static int virtio_ccw_finalize_features(struct virtio_device *vdev)
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
+ /* Give virtio_ccw a chance to accept features. */
+ ccw_transport_features(vdev);
+
features->index = 0;
features->features = cpu_to_le32((u32)vdev->features);
/* Write the first half of the feature bits to the host. */
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index f79a1e17f7d1..807ed4b362c5 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -1968,6 +1968,8 @@ void vring_transport_features(struct virtio_device *vdev)
break;
case VIRTIO_F_IOMMU_PLATFORM:
break;
+ case VIRTIO_F_RING_PACKED:
+ break;
default:
/* We don't understand this bit. */
__virtio_clear_bit(vdev, i);
--
2.18.0
^ permalink raw reply related
* Re: [V9fs-developer] [PATCH] p9_check_errors() validate PDU length
From: jiangyiwen @ 2018-07-11 2:27 UTC (permalink / raw)
To: Tomas Bortoli, ericvh, rminnich, lucho, Andrew Morton
Cc: netdev, linux-kernel, syzkaller, v9fs-developer, davem
In-Reply-To: <20180709224323.20597-1-tomasbortoli@gmail.com>
On 2018/7/10 6:43, Tomas Bortoli wrote:
> p9_check_errors() does not validate the size of the PDU read
> in p9_parse_header(). Any size can be passed, provoking out-of-bound reads.
>
> Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
> Reported-by: syzbot+65c6b72f284a39d416b4@syzkaller.appspotmail.com
> ---
> As suggested by Dominique:
> https://lkml.org/lkml/2018/7/9/688
> Such check is not enough as it will prevent to read more than how it has
> been allocated but it won't prevent to read more than how it has been read
> So this patch will require some more changes to prevent bad sizes.
>
> net/9p/client.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 40f7c47f2f74..5b161b576b8a 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -520,10 +520,13 @@ EXPORT_SYMBOL(p9_parse_header);
> static int p9_check_errors(struct p9_client *c, struct p9_req_t *req)
> {
> int8_t type;
> + int32_t size;
> int err;
> int ecode;
>
> - err = p9_parse_header(req->rc, NULL, &type, NULL, 0);
> + err = p9_parse_header(req->rc, &size, &type, NULL, 0);
> + if (size > req->rc->capacity)
> + return -EINVAL;
Yes, currently 9p client is lacking of properly check for data from server.
Maybe we should have a bigger patch to solve this problem.
> /*
> * dump the response from server
> * This should be after check errors which poplulate pdu_fcall.
>
^ 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