* [PATCH V2 mlx5-next 04/13] net/mlx5: Introduce a helper API to check VF vport
From: Saeed Mahameed @ 2019-06-25 17:47 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Parav Pandit,
Jianbo Liu
In-Reply-To: <20190625174727.20309-1-saeedm@mellanox.com>
From: Parav Pandit <parav@mellanox.com>
Introduce a helper API mlx5_eswitch_is_vf_vport() to check
if a given vport_num belongs to VF or not.
Signed-off-by: Parav Pandit <parav@mellanox.com>
Reviewed-by: Jianbo Liu <jianbol@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/eswitch.h | 2 ++
drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c | 6 ++++++
2 files changed, 8 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index 8b9f2cf58e91..99dc25630629 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -508,6 +508,8 @@ void mlx5e_tc_clean_fdb_peer_flows(struct mlx5_eswitch *esw);
struct mlx5_vport *__must_check
mlx5_eswitch_get_vport(struct mlx5_eswitch *esw, u16 vport_num);
+bool mlx5_eswitch_is_vf_vport(const struct mlx5_eswitch *esw, u16 vport_num);
+
#else /* CONFIG_MLX5_ESWITCH */
/* eswitch API stubs */
static inline int mlx5_eswitch_init(struct mlx5_core_dev *dev) { return 0; }
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 17abb98b48af..c1c42c1370b8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -2290,3 +2290,9 @@ struct mlx5_eswitch_rep *mlx5_eswitch_vport_rep(struct mlx5_eswitch *esw,
return mlx5_eswitch_get_rep(esw, vport);
}
EXPORT_SYMBOL(mlx5_eswitch_vport_rep);
+
+bool mlx5_eswitch_is_vf_vport(const struct mlx5_eswitch *esw, u16 vport_num)
+{
+ return vport_num >= MLX5_VPORT_FIRST_VF &&
+ vport_num <= esw->dev->priv.sriov.max_vfs;
+}
--
2.21.0
^ permalink raw reply related
* [PATCH V2 mlx5-next 03/13] net/mlx5: Support allocating modify header context from ingress ACL
From: Saeed Mahameed @ 2019-06-25 17:47 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Jianbo Liu,
Eli Britstein, Roi Dayan, Mark Bloch
In-Reply-To: <20190625174727.20309-1-saeedm@mellanox.com>
From: Jianbo Liu <jianbol@mellanox.com>
That modify header action can be then attached to a steering rule in
the ingress ACL.
Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
Reviewed-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Reviewed-by: Mark Bloch <markb@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
index bb24c3797218..4f1d402926f1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
@@ -771,6 +771,10 @@ int mlx5_modify_header_alloc(struct mlx5_core_dev *dev,
max_actions = MLX5_CAP_FLOWTABLE_NIC_TX(dev, max_modify_header_actions);
table_type = FS_FT_NIC_TX;
break;
+ case MLX5_FLOW_NAMESPACE_ESW_INGRESS:
+ max_actions = MLX5_CAP_ESW_INGRESS_ACL(dev, max_modify_header_actions);
+ table_type = FS_FT_ESW_INGRESS_ACL;
+ break;
default:
return -EOPNOTSUPP;
}
--
2.21.0
^ permalink raw reply related
* [PATCH V2 mlx5-next 02/13] net/mlx5: Get vport ACL namespace by vport index
From: Saeed Mahameed @ 2019-06-25 17:47 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Jianbo Liu,
Oz Shlomo, Eli Britstein, Roi Dayan, Mark Bloch
In-Reply-To: <20190625174727.20309-1-saeedm@mellanox.com>
From: Jianbo Liu <jianbol@mellanox.com>
The ingress and egress ACL root namespaces are created per vport and
stored into arrays. However, the vport number is not the same as the
index. Passing the array index, instead of vport number, to get the
correct ingress and egress acl namespace.
Fixes: 9b93ab981e3b ("net/mlx5: Separate ingress/egress namespaces for each vport")
Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
Reviewed-by: Oz Shlomo <ozsh@mellanox.com>
Reviewed-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Reviewed-by: Mark Bloch <markb@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index 12010f85fa35..a42a23e505df 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -939,7 +939,7 @@ int esw_vport_enable_egress_acl(struct mlx5_eswitch *esw,
vport->vport, MLX5_CAP_ESW_EGRESS_ACL(dev, log_max_ft_size));
root_ns = mlx5_get_flow_vport_acl_namespace(dev, MLX5_FLOW_NAMESPACE_ESW_EGRESS,
- vport->vport);
+ mlx5_eswitch_vport_num_to_index(esw, vport->vport));
if (!root_ns) {
esw_warn(dev, "Failed to get E-Switch egress flow namespace for vport (%d)\n", vport->vport);
return -EOPNOTSUPP;
@@ -1057,7 +1057,7 @@ int esw_vport_enable_ingress_acl(struct mlx5_eswitch *esw,
vport->vport, MLX5_CAP_ESW_INGRESS_ACL(dev, log_max_ft_size));
root_ns = mlx5_get_flow_vport_acl_namespace(dev, MLX5_FLOW_NAMESPACE_ESW_INGRESS,
- vport->vport);
+ mlx5_eswitch_vport_num_to_index(esw, vport->vport));
if (!root_ns) {
esw_warn(dev, "Failed to get E-Switch ingress flow namespace for vport (%d)\n", vport->vport);
return -EOPNOTSUPP;
--
2.21.0
^ permalink raw reply related
* [PATCH V2 mlx5-next 01/13] net/mlx5: Introduce vport metadata matching bits and enum constants
From: Saeed Mahameed @ 2019-06-25 17:47 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Jianbo Liu,
Eli Britstein, Roi Dayan, Mark Bloch
In-Reply-To: <20190625174727.20309-1-saeedm@mellanox.com>
From: Jianbo Liu <jianbol@mellanox.com>
When a dual-port VHCA sends a RoCE packet on its non-native port, and
the packet arrives to its affiliated vport FDB, a mismatch might occur
on the rules that match the packet source vport. So we replace the
match on source port with the match on metadata that was configured in
ingress ACL, and that metadata will be passed further also to the NIC
RX table of the eswitch manager.
Introduce vport metadata matching bits and enum constants as a pre-step
towards metadata matching.
o metadata type C registers in the misc parameters 2 fields.
o esw_uplink_ingress_acl bit in esw cap. If it set, the device supports
ingress ACL for the uplink vport.
o fdb_to_vport_reg_* bits in flow table cap and esw vport context, to
support propagating the metadata to the nic rx through the loopback
path.
o flow_source in flow context, to indicate the known origin of packets.
o enum constants, to support the above bits.
Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
Reviewed-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Reviewed-by: Mark Bloch <markb@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
include/linux/mlx5/mlx5_ifc.h | 56 ++++++++++++++++++++++++++++++-----
1 file changed, 49 insertions(+), 7 deletions(-)
diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index e3c154b573a2..d4409654f760 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -528,7 +528,21 @@ struct mlx5_ifc_fte_match_set_misc2_bits {
struct mlx5_ifc_fte_match_mpls_bits outer_first_mpls_over_udp;
- u8 reserved_at_80[0x100];
+ u8 metadata_reg_c_7[0x20];
+
+ u8 metadata_reg_c_6[0x20];
+
+ u8 metadata_reg_c_5[0x20];
+
+ u8 metadata_reg_c_4[0x20];
+
+ u8 metadata_reg_c_3[0x20];
+
+ u8 metadata_reg_c_2[0x20];
+
+ u8 metadata_reg_c_1[0x20];
+
+ u8 metadata_reg_c_0[0x20];
u8 metadata_reg_a[0x20];
@@ -636,8 +650,22 @@ struct mlx5_ifc_flow_table_nic_cap_bits {
u8 reserved_at_e00[0x7200];
};
+enum {
+ MLX5_FDB_TO_VPORT_REG_C_0 = 0x01,
+ MLX5_FDB_TO_VPORT_REG_C_1 = 0x02,
+ MLX5_FDB_TO_VPORT_REG_C_2 = 0x04,
+ MLX5_FDB_TO_VPORT_REG_C_3 = 0x08,
+ MLX5_FDB_TO_VPORT_REG_C_4 = 0x10,
+ MLX5_FDB_TO_VPORT_REG_C_5 = 0x20,
+ MLX5_FDB_TO_VPORT_REG_C_6 = 0x40,
+ MLX5_FDB_TO_VPORT_REG_C_7 = 0x80,
+};
+
struct mlx5_ifc_flow_table_eswitch_cap_bits {
- u8 reserved_at_0[0x1a];
+ u8 fdb_to_vport_reg_c_id[0x8];
+ u8 reserved_at_8[0xf];
+ u8 flow_source[0x1];
+ u8 reserved_at_18[0x2];
u8 multi_fdb_encap[0x1];
u8 reserved_at_1b[0x1];
u8 fdb_multi_path_to_table[0x1];
@@ -665,7 +693,9 @@ struct mlx5_ifc_e_switch_cap_bits {
u8 vport_svlan_insert[0x1];
u8 vport_cvlan_insert_if_not_exist[0x1];
u8 vport_cvlan_insert_overwrite[0x1];
- u8 reserved_at_5[0x14];
+ u8 reserved_at_5[0x3];
+ u8 esw_uplink_ingress_acl[0x1];
+ u8 reserved_at_9[0x10];
u8 esw_functions_changed[0x1];
u8 reserved_at_1a[0x1];
u8 ecpf_vport_exists[0x1];
@@ -2555,6 +2585,12 @@ enum {
MLX5_FLOW_CONTEXT_ACTION_VLAN_PUSH_2 = 0x800,
};
+enum {
+ MLX5_FLOW_CONTEXT_FLOW_SOURCE_ANY_VPORT = 0x0,
+ MLX5_FLOW_CONTEXT_FLOW_SOURCE_UPLINK = 0x1,
+ MLX5_FLOW_CONTEXT_FLOW_SOURCE_LOCAL_VPORT = 0x2,
+};
+
struct mlx5_ifc_vlan_bits {
u8 ethtype[0x10];
u8 prio[0x3];
@@ -2574,7 +2610,9 @@ struct mlx5_ifc_flow_context_bits {
u8 action[0x10];
u8 extended_destination[0x1];
- u8 reserved_at_80[0x7];
+ u8 reserved_at_81[0x1];
+ u8 flow_source[0x2];
+ u8 reserved_at_84[0x4];
u8 destination_list_size[0x18];
u8 reserved_at_a0[0x8];
@@ -3099,12 +3137,14 @@ struct mlx5_ifc_hca_vport_context_bits {
};
struct mlx5_ifc_esw_vport_context_bits {
- u8 reserved_at_0[0x3];
+ u8 fdb_to_vport_reg_c[0x1];
+ u8 reserved_at_1[0x2];
u8 vport_svlan_strip[0x1];
u8 vport_cvlan_strip[0x1];
u8 vport_svlan_insert[0x1];
u8 vport_cvlan_insert[0x2];
- u8 reserved_at_8[0x18];
+ u8 fdb_to_vport_reg_c_id[0x8];
+ u8 reserved_at_10[0x10];
u8 reserved_at_20[0x20];
@@ -4985,7 +5025,8 @@ struct mlx5_ifc_modify_esw_vport_context_out_bits {
};
struct mlx5_ifc_esw_vport_context_fields_select_bits {
- u8 reserved_at_0[0x1c];
+ u8 reserved_at_0[0x1b];
+ u8 fdb_to_vport_reg_c_id[0x1];
u8 vport_cvlan_insert[0x1];
u8 vport_svlan_insert[0x1];
u8 vport_cvlan_strip[0x1];
@@ -5182,6 +5223,7 @@ enum {
MLX5_ACTION_IN_FIELD_OUT_DIPV4 = 0x16,
MLX5_ACTION_IN_FIELD_OUT_FIRST_VID = 0x17,
MLX5_ACTION_IN_FIELD_OUT_IPV6_HOPLIMIT = 0x47,
+ MLX5_ACTION_IN_FIELD_METADATA_REG_C_0 = 0x51,
};
struct mlx5_ifc_alloc_modify_header_context_out_bits {
--
2.21.0
^ permalink raw reply related
* [PATCH V2 mlx5-next 00/13] Mellanox, mlx5 vport metadata matching
From: Saeed Mahameed @ 2019-06-25 17:47 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org
This series includes mlx5 updates for both rdma and net-next trees.
In case of no objection it will be applied to mlx5-next branch and later
on will be sent as pull request to rdma and net-next.
From Jianbo, Vport meta data matching:
Hardware steering has no notion of vport number, and vport is an
abstract concept, so firmware need to translate the source vport
matching to match on the VHCA ID (Virtual HCA ID).
In dual-port RoCE, the dual-port VHCA is able to send also on the
second port on behalf of the affiliated vport, so now we can’t assume
anymore that vport is represented by single VHCA only.
To resolve this issue, we use metadata register as source port
indicator instead.
When a packet enters the eswitch, eswitch ingress traffic passes the
ingress ACL flow tables, where we tag the packets (via the metadata
value, in this case REG_C at index 0) with a unique value which will
act as an alias of the vport. In order to guarantee uniqueness, we use
the eswitch owner vhca id and the vport number as that value.
Usually, the vports are numbered in each eswitch as followed:
- Physical Function (PF) vport, the number is 0.
- Virtual Function (VF) vport, starting from 1.
- Uplink vport, the reserved vport number for it is 0xFFFF.
With the metadata in each packet, we can then do matching on it, in
both fast path and slow path.
For slow path, there is a representor for each vport. Packet that
misses all offloaded rules in FDB, will be forwarded to the eswitch
manager vport. In its NIC RX, it then will be steered to the right
representor. The rules, which decide the destination representor,
previously were matching on source port, will now match metadata
instead.
V2:
- Remove eswitch cleanup patches from bodong, will submit later.
- Remove IB specified APIs (mlx5_ib_eswitch_*) added in V1.
- Add mlx5_eswitch_is_vf_vport() to check if the vport is VF vport.
- Other small changes.
Thanks,
Saeed.
---
Jianbo Liu (12):
net/mlx5: Introduce vport metadata matching bits and enum constants
net/mlx5: Get vport ACL namespace by vport index
net/mlx5: Support allocating modify header context from ingress ACL
net/mlx5: Add flow context for flow tag
net/mlx5: E-Switch, Tag packet with vport number in VF vports and
uplink ingress ACLs
net/mlx5e: Specifying known origin of packets matching the flow
net/mlx5: E-Switch, Add match on vport metadata for rule in fast path
net/mlx5: E-Switch, Add query and modify esw vport context functions
net/mlx5: E-Switch, Pass metadata from FDB to eswitch manager
net/mlx5: E-Switch, Add match on vport metadata for rule in slow path
RDMA/mlx5: Add vport metadata matching for IB representors
net/mlx5: E-Switch, Enable vport metadata matching if firmware
supports it
Parav Pandit (1):
net/mlx5: Introduce a helper API to check VF vport
drivers/infiniband/hw/mlx5/flow.c | 13 +-
drivers/infiniband/hw/mlx5/main.c | 75 ++-
drivers/infiniband/hw/mlx5/mlx5_ib.h | 1 +
.../mellanox/mlx5/core/diag/fs_tracepoint.h | 4 +-
.../mellanox/mlx5/core/en_fs_ethtool.c | 2 +-
.../net/ethernet/mellanox/mlx5/core/en_tc.c | 7 +-
.../net/ethernet/mellanox/mlx5/core/eswitch.c | 30 +-
.../net/ethernet/mellanox/mlx5/core/eswitch.h | 16 +
.../mellanox/mlx5/core/eswitch_offloads.c | 500 ++++++++++++++----
.../ethernet/mellanox/mlx5/core/fpga/ipsec.c | 8 +-
.../net/ethernet/mellanox/mlx5/core/fs_cmd.c | 10 +-
.../net/ethernet/mellanox/mlx5/core/fs_core.c | 34 +-
.../net/ethernet/mellanox/mlx5/core/fs_core.h | 1 +
include/linux/mlx5/eswitch.h | 17 +
include/linux/mlx5/fs.h | 16 +-
include/linux/mlx5/mlx5_ifc.h | 56 +-
16 files changed, 622 insertions(+), 168 deletions(-)
--
2.21.0
^ permalink raw reply
* Re: [PATCH net-next] tools: lib: bpf: libbpf: fix max() type mistmatch for 32bit
From: Andrii Nakryiko @ 2019-06-25 17:45 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: Alexei Starovoitov, Networking, Daniel Borkmann, bpf, open list
In-Reply-To: <20190625141142.2378-1-ivan.khoronzhuk@linaro.org>
On Tue, Jun 25, 2019 at 7:12 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> It fixes build error for 32bit coused by type mistmatch
typo: coused -> caused, mistmatch -> mismatch
> size_t/unsigned long.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>
> Based on net-next/master
libbpf changes should be based against bpf-next. Indicate that also
with [PATCH bpf-next] subject prefix.
Also, there is no need to have this long "tools: lib: bpf: libbpf:"
prefix, it's not supposed to be a repeat of file path. Use just
"libbpf: " to indicate it's libbpf-related changes.
>
> tools/lib/bpf/libbpf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 4259c9f0cfe7..d03016a559e2 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -778,7 +778,7 @@ static struct bpf_map *bpf_object__add_map(struct bpf_object *obj)
> if (obj->nr_maps < obj->maps_cap)
> return &obj->maps[obj->nr_maps++];
>
> - new_cap = max(4ul, obj->maps_cap * 3 / 2);
> + new_cap = max((size_t)4, obj->maps_cap * 3 / 2);
The fix itself works, thanks!
> new_maps = realloc(obj->maps, new_cap * sizeof(*obj->maps));
> if (!new_maps) {
> pr_warning("alloc maps for object failed\n");
> --
> 2.17.1
>
^ permalink raw reply
* [net-next 1/1] tipc: rename function msg_get_wrapped() to msg_inner_hdr()
From: Jon Maloy @ 2019-06-25 17:37 UTC (permalink / raw)
To: davem, netdev
Cc: gordan.mihaljevic, tung.q.nguyen, hoang.h.le, jon.maloy,
canh.d.luu, ying.xue, tipc-discussion
We rename the inline function msg_get_wrapped() to the more
comprehensible msg_inner_hdr().
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/bcast.c | 4 ++--
net/tipc/link.c | 2 +-
net/tipc/msg.h | 4 ++--
net/tipc/node.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 6c997d4..1336f3c 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -323,7 +323,7 @@ static int tipc_mcast_send_sync(struct net *net, struct sk_buff *skb,
hdr = buf_msg(skb);
if (msg_user(hdr) == MSG_FRAGMENTER)
- hdr = msg_get_wrapped(hdr);
+ hdr = msg_inner_hdr(hdr);
if (msg_type(hdr) != TIPC_MCAST_MSG)
return 0;
@@ -392,7 +392,7 @@ int tipc_mcast_xmit(struct net *net, struct sk_buff_head *pkts,
skb = skb_peek(pkts);
hdr = buf_msg(skb);
if (msg_user(hdr) == MSG_FRAGMENTER)
- hdr = msg_get_wrapped(hdr);
+ hdr = msg_inner_hdr(hdr);
msg_set_is_rcast(hdr, method->rcast);
/* Switch method ? */
diff --git a/net/tipc/link.c b/net/tipc/link.c
index aa79bf8..f8bf63b 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -732,7 +732,7 @@ static void link_profile_stats(struct tipc_link *l)
if (msg_user(msg) == MSG_FRAGMENTER) {
if (msg_type(msg) != FIRST_FRAGMENT)
return;
- length = msg_size(msg_get_wrapped(msg));
+ length = msg_size(msg_inner_hdr(msg));
}
l->stats.msg_lengths_total += length;
l->stats.msg_length_counts++;
diff --git a/net/tipc/msg.h b/net/tipc/msg.h
index 8de02ad..da509f0 100644
--- a/net/tipc/msg.h
+++ b/net/tipc/msg.h
@@ -308,7 +308,7 @@ static inline unchar *msg_data(struct tipc_msg *m)
return ((unchar *)m) + msg_hdr_sz(m);
}
-static inline struct tipc_msg *msg_get_wrapped(struct tipc_msg *m)
+static inline struct tipc_msg *msg_inner_hdr(struct tipc_msg *m)
{
return (struct tipc_msg *)msg_data(m);
}
@@ -486,7 +486,7 @@ static inline void msg_set_prevnode(struct tipc_msg *m, u32 a)
static inline u32 msg_origport(struct tipc_msg *m)
{
if (msg_user(m) == MSG_FRAGMENTER)
- m = msg_get_wrapped(m);
+ m = msg_inner_hdr(m);
return msg_word(m, 4);
}
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 550581d..324a1f9 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1649,7 +1649,7 @@ static bool tipc_node_check_state(struct tipc_node *n, struct sk_buff *skb,
int usr = msg_user(hdr);
int mtyp = msg_type(hdr);
u16 oseqno = msg_seqno(hdr);
- u16 iseqno = msg_seqno(msg_get_wrapped(hdr));
+ u16 iseqno = msg_seqno(msg_inner_hdr(hdr));
u16 exp_pkts = msg_msgcnt(hdr);
u16 rcv_nxt, syncpt, dlv_nxt, inputq_len;
int state = n->state;
--
2.1.4
^ permalink raw reply related
* [PATCH bpf-next v4 2/2] bpf: Add selftests for bpf_perf_event_output
From: allanzhang @ 2019-06-25 17:27 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, David S. Miller, netdev, bpf, linux-kernel
Cc: allanzhang
In-Reply-To: <20190625172717.158613-1-allanzhang@google.com>
Software event output is only enabled by a few prog types.
This test is to ensure that all supported types are enbled for
bpf_perf_event_output sucessfully.
v4:
* Reformating log message
v3:
* Reformating log message
v2:
* Reformating log message
Signed-off-by: allanzhang <allanzhang@google.com>
---
tools/testing/selftests/bpf/test_verifier.c | 33 ++++++-
.../selftests/bpf/verifier/event_output.c | 94 +++++++++++++++++++
2 files changed, 126 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/verifier/event_output.c
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index c5514daf8865..901a188e1eea 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -50,7 +50,7 @@
#define MAX_INSNS BPF_MAXINSNS
#define MAX_TEST_INSNS 1000000
#define MAX_FIXUPS 8
-#define MAX_NR_MAPS 18
+#define MAX_NR_MAPS 19
#define MAX_TEST_RUNS 8
#define POINTER_VALUE 0xcafe4all
#define TEST_DATA_LEN 64
@@ -84,6 +84,7 @@ struct bpf_test {
int fixup_map_array_wo[MAX_FIXUPS];
int fixup_map_array_small[MAX_FIXUPS];
int fixup_sk_storage_map[MAX_FIXUPS];
+ int fixup_map_event_output[MAX_FIXUPS];
const char *errstr;
const char *errstr_unpriv;
uint32_t retval, retval_unpriv, insn_processed;
@@ -604,6 +605,28 @@ static int create_sk_storage_map(void)
return fd;
}
+static int create_event_output_map(void)
+{
+ struct bpf_create_map_attr attr = {
+ .name = "test_map",
+ .map_type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
+ .key_size = 4,
+ .value_size = 4,
+ .max_entries = 1,
+ };
+ int fd, btf_fd;
+
+ btf_fd = load_btf();
+ if (btf_fd < 0)
+ return -1;
+ attr.btf_fd = btf_fd;
+ fd = bpf_create_map_xattr(&attr);
+ close(attr.btf_fd);
+ if (fd < 0)
+ printf("Failed to create event_output\n");
+ return fd;
+}
+
static char bpf_vlog[UINT_MAX >> 8];
static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
@@ -627,6 +650,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
int *fixup_map_array_wo = test->fixup_map_array_wo;
int *fixup_map_array_small = test->fixup_map_array_small;
int *fixup_sk_storage_map = test->fixup_sk_storage_map;
+ int *fixup_map_event_output = test->fixup_map_event_output;
if (test->fill_helper) {
test->fill_insns = calloc(MAX_TEST_INSNS, sizeof(struct bpf_insn));
@@ -788,6 +812,13 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
fixup_sk_storage_map++;
} while (*fixup_sk_storage_map);
}
+ if (*fixup_map_event_output) {
+ map_fds[18] = create_event_output_map();
+ do {
+ prog[*fixup_map_event_output].imm = map_fds[18];
+ fixup_map_event_output++;
+ } while (*fixup_map_event_output);
+ }
}
static int set_admin(bool admin)
diff --git a/tools/testing/selftests/bpf/verifier/event_output.c b/tools/testing/selftests/bpf/verifier/event_output.c
new file mode 100644
index 000000000000..b25eabcfaa56
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/event_output.c
@@ -0,0 +1,94 @@
+/* instructions used to output a skb based software event, produced
+ * from code snippet:
+struct TMP {
+ uint64_t tmp;
+} tt;
+tt.tmp = 5;
+bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
+ &tt, sizeof(tt));
+return 1;
+
+the bpf assembly from llvm is:
+ 0: b7 02 00 00 05 00 00 00 r2 = 5
+ 1: 7b 2a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r2
+ 2: bf a4 00 00 00 00 00 00 r4 = r10
+ 3: 07 04 00 00 f8 ff ff ff r4 += -8
+ 4: 18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0ll
+ 6: b7 03 00 00 00 00 00 00 r3 = 0
+ 7: b7 05 00 00 08 00 00 00 r5 = 8
+ 8: 85 00 00 00 19 00 00 00 call 25
+ 9: b7 00 00 00 01 00 00 00 r0 = 1
+ 10: 95 00 00 00 00 00 00 00 exit
+
+ The reason I put the code here instead of fill_helpers is that map fixup is
+ against the insns, instead of filled prog.
+*/
+
+#define __PERF_EVENT_INSNS__ \
+ BPF_MOV64_IMM(BPF_REG_2, 5), \
+ BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -8), \
+ BPF_MOV64_REG(BPF_REG_4, BPF_REG_10), \
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -8), \
+ BPF_LD_MAP_FD(BPF_REG_2, 0), \
+ BPF_MOV64_IMM(BPF_REG_3, 0), \
+ BPF_MOV64_IMM(BPF_REG_5, 8), \
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, \
+ BPF_FUNC_perf_event_output), \
+ BPF_MOV64_IMM(BPF_REG_0, 1), \
+ BPF_EXIT_INSN(),
+{
+ "perfevent for sockops",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SOCK_OPS,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for tc",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for lwt out",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_LWT_OUT,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for xdp",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_XDP,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for socket filter",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SOCKET_FILTER,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for sk_skb",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SK_SKB,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for cgroup skb",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v4 1/2] bpf: Allow bpf_skb_event_output for a few prog types
From: allanzhang @ 2019-06-25 17:27 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, David S. Miller, netdev, bpf, linux-kernel
Cc: allanzhang
In-Reply-To: <20190625172717.158613-1-allanzhang@google.com>
Software event output is only enabled by a few prog types right now (TC,
LWT out, XDP, sockops). Many other skb based prog types need
bpf_skb_event_output to produce software event.
Added socket_filter, cg_skb, sk_skb prog types to generate sw event.
Test bpf code is generated from code snippet:
struct TMP {
uint64_t tmp;
} tt;
tt.tmp = 5;
bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
&tt, sizeof(tt));
return 1;
the bpf assembly from llvm is:
0: b7 02 00 00 05 00 00 00 r2 = 5
1: 7b 2a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r2
2: bf a4 00 00 00 00 00 00 r4 = r10
3: 07 04 00 00 f8 ff ff ff r4 += -8
4: 18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0ll
6: b7 03 00 00 00 00 00 00 r3 = 0
7: b7 05 00 00 08 00 00 00 r5 = 8
8: 85 00 00 00 19 00 00 00 call 25
9: b7 00 00 00 01 00 00 00 r0 = 1
10: 95 00 00 00 00 00 00 00 exit
Signed-off-by: allanzhang <allanzhang@google.com>
v4:
* Reformating log message
v3:
* Reformating log message
v2:
* Reformating log message
---
net/core/filter.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/core/filter.c b/net/core/filter.c
index 2014d76e0d2a..b75fcf412628 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5958,6 +5958,8 @@ sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_get_socket_cookie_proto;
case BPF_FUNC_get_socket_uid:
return &bpf_get_socket_uid_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_skb_event_output_proto;
default:
return bpf_base_func_proto(func_id);
}
@@ -5978,6 +5980,8 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sk_storage_get_proto;
case BPF_FUNC_sk_storage_delete:
return &bpf_sk_storage_delete_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_skb_event_output_proto;
#ifdef CONFIG_SOCK_CGROUP_DATA
case BPF_FUNC_skb_cgroup_id:
return &bpf_skb_cgroup_id_proto;
@@ -6226,6 +6230,8 @@ sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sk_redirect_map_proto;
case BPF_FUNC_sk_redirect_hash:
return &bpf_sk_redirect_hash_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_skb_event_output_proto;
#ifdef CONFIG_INET
case BPF_FUNC_sk_lookup_tcp:
return &bpf_sk_lookup_tcp_proto;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v4 0/2] bpf: Allow bpf_skb_event_output for more prog types
From: allanzhang @ 2019-06-25 17:27 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, David S. Miller, netdev, bpf, linux-kernel
Cc: allanzhang
Software event output is only enabled by a few prog types right now (TC,
LWT out, XDP, sockops). Many other skb based prog types need
bpf_skb_event_output to produce software event.
Added socket_filter, cg_skb, sk_skb prog types to generate sw event.
allanzhang (2):
bpf: Allow bpf_skb_event_output for a few prog types
bpf: Add selftests for bpf_perf_event_output
net/core/filter.c | 6 ++
tools/testing/selftests/bpf/test_verifier.c | 33 ++++++-
.../selftests/bpf/verifier/event_output.c | 94 +++++++++++++++++++
3 files changed, 132 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/verifier/event_output.c
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* Re: [PATCH V3 12/15] ARM: vexpress: cleanup cppcheck shifting error
From: Sudeep Holla @ 2019-06-25 17:24 UTC (permalink / raw)
To: Phong Tran
Cc: acme, alexander.shishkin, alexander.sverdlin, allison, andrew,
ast, bgolaszewski, bpf, daniel, daniel, dmg, festevam, gerg,
gregkh, gregory.clement, haojian.zhuang, hsweeten,
illusionist.neo, info, jason, jolsa, kafai, kernel, kgene, krzk,
kstewart, linux-arm-kernel, linux-imx, linux-kernel, linux-omap,
linux-samsung-soc, linux, liviu.dudau, lkundrak,
lorenzo.pieralisi, mark.rutland, mingo, namhyung, netdev, nsekhar,
peterz, robert.jarzmik, s.hauer, sebastian.hesselbarth, shawnguo,
songliubraving, swinslow, tglx, tony, will, yhs, Sudeep Holla
In-Reply-To: <20190625040356.27473-13-tranmanphong@gmail.com>
On Tue, Jun 25, 2019 at 11:03:53AM +0700, Phong Tran wrote:
> There is error from cppcheck tool
> "Shifting signed 32-bit value by 31 bits is undefined behaviour errors"
> change to use BIT() marco for improvement.
>
What's your plan for merging this series ? I can take this for v5.4
If not,
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH bpf] tools: bpftool: use correct argument in cgroup errors
From: Roman Gushchin @ 2019-06-25 17:21 UTC (permalink / raw)
To: Jakub Kicinski
Cc: alexei.starovoitov@gmail.com, daniel@iogearbox.net,
netdev@vger.kernel.org, bpf@vger.kernel.org,
oss-drivers@netronome.com, sdf@google.com, Quentin Monnet
In-Reply-To: <20190625165631.18928-1-jakub.kicinski@netronome.com>
On Tue, Jun 25, 2019 at 09:56:31AM -0700, Jakub Kicinski wrote:
> cgroup code tries to use argv[0] as the cgroup path,
> but if it fails uses argv[1] to report errors.
>
> Fixes: 5ccda64d38cc ("bpftool: implement cgroup bpf operations")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Roman Gushchin <guro@fb.com>
Thanks, Jakub!
> ---
> tools/bpf/bpftool/cgroup.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> index 73ec8ea33fb4..a13fb7265d1a 100644
> --- a/tools/bpf/bpftool/cgroup.c
> +++ b/tools/bpf/bpftool/cgroup.c
> @@ -168,7 +168,7 @@ static int do_show(int argc, char **argv)
>
> cgroup_fd = open(argv[0], O_RDONLY);
> if (cgroup_fd < 0) {
> - p_err("can't open cgroup %s", argv[1]);
> + p_err("can't open cgroup %s", argv[0]);
> goto exit;
> }
>
> @@ -356,7 +356,7 @@ static int do_attach(int argc, char **argv)
>
> cgroup_fd = open(argv[0], O_RDONLY);
> if (cgroup_fd < 0) {
> - p_err("can't open cgroup %s", argv[1]);
> + p_err("can't open cgroup %s", argv[0]);
> goto exit;
> }
>
> @@ -414,7 +414,7 @@ static int do_detach(int argc, char **argv)
>
> cgroup_fd = open(argv[0], O_RDONLY);
> if (cgroup_fd < 0) {
> - p_err("can't open cgroup %s", argv[1]);
> + p_err("can't open cgroup %s", argv[0]);
> goto exit;
> }
>
> --
> 2.21.0
>
^ permalink raw reply
* Re: [PATCH V3 01/15] arm: perf: cleanup cppcheck shifting error
From: Will Deacon @ 2019-06-25 17:21 UTC (permalink / raw)
To: Phong Tran
Cc: acme, alexander.shishkin, alexander.sverdlin, allison, andrew,
ast, bgolaszewski, bpf, daniel, daniel, dmg, festevam, gerg,
gregkh, gregory.clement, haojian.zhuang, hsweeten,
illusionist.neo, info, jason, jolsa, kafai, kernel, kgene, krzk,
kstewart, linux-arm-kernel, linux-imx, linux-kernel, linux-omap,
linux-samsung-soc, linux, liviu.dudau, lkundrak,
lorenzo.pieralisi, mark.rutland, mingo, namhyung, netdev, nsekhar,
peterz, robert.jarzmik, s.hauer, sebastian.hesselbarth, shawnguo,
songliubraving, sudeep.holla, swinslow, tglx, tony, yhs
In-Reply-To: <20190625040356.27473-2-tranmanphong@gmail.com>
On Tue, Jun 25, 2019 at 11:03:42AM +0700, Phong Tran wrote:
> There is error from cppcheck tool
> "Shifting signed 32-bit value by 31 bits is undefined behaviour errors"
> change to use BIT() marco for improvement.
s/marco/macro/
As Peter pointed out, this "error" is also a false positive also for the
kernel.
> Signed-off-by: Phong Tran <tranmanphong@gmail.com>
> ---
> arch/arm/kernel/perf_event_v7.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/kernel/perf_event_v7.c b/arch/arm/kernel/perf_event_v7.c
> index a4fb0f8b8f84..2924d7910b10 100644
> --- a/arch/arm/kernel/perf_event_v7.c
> +++ b/arch/arm/kernel/perf_event_v7.c
> @@ -697,9 +697,9 @@ static struct attribute_group armv7_pmuv2_events_attr_group = {
> /*
> * Event filters for PMUv2
> */
> -#define ARMV7_EXCLUDE_PL1 (1 << 31)
> -#define ARMV7_EXCLUDE_USER (1 << 30)
> -#define ARMV7_INCLUDE_HYP (1 << 27)
> +#define ARMV7_EXCLUDE_PL1 BIT(31)
> +#define ARMV7_EXCLUDE_USER BIT(30)
> +#define ARMV7_INCLUDE_HYP BIT(27)
Acked-by: Will Deacon <will.deacon@arm.com>
You can drop this into Russell's patch system[1].
Will
[1] https://www.arm.linux.org.uk/developer/patches/
^ permalink raw reply
* Re: [PATCH] perf cs-etm: Improve completeness for kernel address space
From: Mathieu Poirier @ 2019-06-25 17:14 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Leo Yan, Linux Kernel Mailing List, linux-arm-kernel, netdev, bpf,
Alexander Shishkin, Jiri Olsa, Namhyung Kim, Peter Zijlstra,
Suzuki Poulouse, Coresight ML
In-Reply-To: <20190624190009.GE4181@kernel.org>
On Mon, 24 Jun 2019 at 13:00, Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Thu, Jun 20, 2019 at 08:58:29AM +0800, Leo Yan escreveu:
> > Hi Mathieu,
> >
> > On Wed, Jun 19, 2019 at 11:49:44AM -0600, Mathieu Poirier wrote:
> >
> > [...]
> >
> > > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > > > index 51dd00f65709..4776c2c1fb6d 100644
> > > > --- a/tools/perf/Makefile.config
> > > > +++ b/tools/perf/Makefile.config
> > > > @@ -418,6 +418,30 @@ ifdef CORESIGHT
> > > > endif
> > > > LDFLAGS += $(LIBOPENCSD_LDFLAGS)
> > > > EXTLIBS += $(OPENCSDLIBS)
> > > > + ifneq ($(wildcard $(srctree)/arch/arm64/kernel/vmlinux.lds),)
> > > > + # Extract info from lds:
> > > > + # . = ((((((((0xffffffffffffffff)) - (((1)) << (48)) + 1) + (0)) + (0x08000000))) + (0x08000000))) + 0x00080000;
> > > > + # ARM64_PRE_START_SIZE := (0x08000000 + 0x08000000 + 0x00080000)
> > > > + ARM64_PRE_START_SIZE := $(shell egrep ' \. \= \({8}0x[0-9a-fA-F]+\){2}' \
> > > > + $(srctree)/arch/arm64/kernel/vmlinux.lds | \
> > > > + sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> > > > + awk -F' ' '{print "("$$6 "+" $$7 "+" $$8")"}' 2>/dev/null)
> > > > + else
> > > > + ARM64_PRE_START_SIZE := 0
> > > > + endif
> > > > + CFLAGS += -DARM64_PRE_START_SIZE="$(ARM64_PRE_START_SIZE)"
> > > > + ifneq ($(wildcard $(srctree)/arch/arm/kernel/vmlinux.lds),)
> > > > + # Extract info from lds:
> > > > + # . = ((0xC0000000)) + 0x00208000;
> > > > + # ARM_PRE_START_SIZE := 0x00208000
> > > > + ARM_PRE_START_SIZE := $(shell egrep ' \. \= \({2}0x[0-9a-fA-F]+\){2}' \
> > > > + $(srctree)/arch/arm/kernel/vmlinux.lds | \
> > > > + sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> > > > + awk -F' ' '{print "("$$2")"}' 2>/dev/null)
> > > > + else
> > > > + ARM_PRE_START_SIZE := 0
> > > > + endif
> > > > + CFLAGS += -DARM_PRE_START_SIZE="$(ARM_PRE_START_SIZE)"
> > > > $(call detected,CONFIG_LIBOPENCSD)
> > > > ifdef CSTRACE_RAW
> > > > CFLAGS += -DCS_DEBUG_RAW
> > > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > > > index 0c7776b51045..ae831f836c70 100644
> > > > --- a/tools/perf/util/cs-etm.c
> > > > +++ b/tools/perf/util/cs-etm.c
> > > > @@ -613,10 +613,34 @@ static void cs_etm__free(struct perf_session *session)
> > > > static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address)
> > > > {
> > > > struct machine *machine;
> > > > + u64 fixup_kernel_start = 0;
> > > > + const char *arch;
> > > >
> > > > machine = etmq->etm->machine;
> > > > + arch = perf_env__arch(machine->env);
> > > >
> > > > - if (address >= etmq->etm->kernel_start) {
> > > > + /*
> > > > + * Since arm and arm64 specify some memory regions prior to
> > > > + * 'kernel_start', kernel addresses can be less than 'kernel_start'.
> > > > + *
> > > > + * For arm architecture, the 16MB virtual memory space prior to
> > > > + * 'kernel_start' is allocated to device modules, a PMD table if
> > > > + * CONFIG_HIGHMEM is enabled and a PGD table.
> > > > + *
> > > > + * For arm64 architecture, the root PGD table, device module memory
> > > > + * region and BPF jit region are prior to 'kernel_start'.
> > > > + *
> > > > + * To reflect the complete kernel address space, compensate these
> > > > + * pre-defined regions for kernel start address.
> > > > + */
> > > > + if (!strcmp(arch, "arm64"))
> > > > + fixup_kernel_start = etmq->etm->kernel_start -
> > > > + ARM64_PRE_START_SIZE;
> > > > + else if (!strcmp(arch, "arm"))
> > > > + fixup_kernel_start = etmq->etm->kernel_start -
> > > > + ARM_PRE_START_SIZE;
> > >
> > > I will test your work but from a quick look wouldn't it be better to
> > > have a single define name here? From looking at the modifications you
> > > did to Makefile.config there doesn't seem to be a reason to have two.
> >
> > Thanks for suggestion. I changed to use single define
> > ARM_PRE_START_SIZE and sent patch v2 [1].
> >
> > If possible, please test patch v2.
> >
> > Thanks,
> > Leo Yan
>
> So just for the record, I'm waiting for Mathieu on this one, i.e. for
> him to test/ack v3.
Right, please give me some time to test this. As Leo indicated the
procedure is time consuming.
Thanks,
Mathieu
>
> - Arnaldo
>
> > [1] https://lore.kernel.org/linux-arm-kernel/20190620005428.20883-1-leo.yan@linaro.org/T/#u
> >
> > > > +
> > > > + if (address >= fixup_kernel_start) {
> > > > if (machine__is_host(machine))
> > > > return PERF_RECORD_MISC_KERNEL;
> > > > else
> > > > --
> > > > 2.17.1
> > > >
>
> --
>
> - Arnaldo
^ permalink raw reply
* Re: Warnings generated from tcp_sacktag_write_queue.
From: Eric Dumazet @ 2019-06-25 17:10 UTC (permalink / raw)
To: Chinmay Agarwal, netdev; +Cc: sharathv, kapandey
In-Reply-To: <20190625155734.GA31551@chinagar-linux.qualcomm.com>
On 6/25/19 8:57 AM, Chinmay Agarwal wrote:
>
> The kernel version used is 4.14.
>
Do not use this old version please.
^ permalink raw reply
* Re: Removing skb_orphan() from ip_rcv_core()
From: Eric Dumazet @ 2019-06-25 17:03 UTC (permalink / raw)
To: Daniel Borkmann, Joe Stringer, Florian Westphal
Cc: netdev, john fastabend, Lorenz Bauer, Jakub Sitnicki, Paolo Abeni,
Flavio Leitner, ast
In-Reply-To: <4deff7d7-cc10-090d-86f2-850148fdf032@iogearbox.net>
On 6/25/19 2:35 AM, Daniel Borkmann wrote:
>
> But wasn't the whole point of 9c4c325252c5 ("skbuff: preserve sock reference when
> scrubbing the skb.") to defer orphaning to as late as possible? If I'm not missing
> anything, then above would reintroduce the issues that 9c4c325252c5 was trying to
> solve wrt TSQ/XPS/etc when skb was sent via veth based data path to cross netns and
> then forwarded to phys dev for transmission; meaning, skb->sk is lost at the point
> of dev_queue_xmit() for the latter. A side-effect this would also have is that this
> changes behavior again for tc egress programs sitting on phys dev (e.g. querying
> sock cookie or other related features).
Unless we can detect/decide that a packet going through veth pair is going to be locally
consumed, or forwarded to a physical device (another ndo_start_xmit()), we need
to skb_orphan() the packet, exactly the same way than loopback ndo_start_xmit()
(We could have setups where these packets going through lo interface could be forwarded
to a NIC...)
Backpressure is a best effort, we should not make it an absolute requirement and
prevent doing early demux as early as possible in RX path.
^ permalink raw reply
* [PATCH net-next] Revert "net: ena: ethtool: add extra properties retrieval via get_priv_flags"
From: Jakub Kicinski @ 2019-06-25 16:59 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, nafea, dwmw2, sameehj, zorik, saeedb,
netanel, Jakub Kicinski
This reverts commit 315c28d2b714 ("net: ena: ethtool: add extra properties retrieval via get_priv_flags").
As discussed at netconf and on the mailing list we can't allow
for the the abuse of private flags for exposing arbitrary device
labels.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../net/ethernet/amazon/ena/ena_admin_defs.h | 16 ----
drivers/net/ethernet/amazon/ena/ena_com.c | 56 --------------
drivers/net/ethernet/amazon/ena/ena_com.h | 32 --------
drivers/net/ethernet/amazon/ena/ena_ethtool.c | 75 +++----------------
drivers/net/ethernet/amazon/ena/ena_netdev.c | 14 ----
drivers/net/ethernet/amazon/ena/ena_netdev.h | 2 -
6 files changed, 11 insertions(+), 184 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_admin_defs.h b/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
index c8638f7b5b8e..d19f2ecf8e84 100644
--- a/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
+++ b/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
@@ -32,8 +32,6 @@
#ifndef _ENA_ADMIN_H_
#define _ENA_ADMIN_H_
-#define ENA_ADMIN_EXTRA_PROPERTIES_STRING_LEN 32
-#define ENA_ADMIN_EXTRA_PROPERTIES_COUNT 32
enum ena_admin_aq_opcode {
ENA_ADMIN_CREATE_SQ = 1,
@@ -62,8 +60,6 @@ enum ena_admin_aq_feature_id {
ENA_ADMIN_MAX_QUEUES_NUM = 2,
ENA_ADMIN_HW_HINTS = 3,
ENA_ADMIN_LLQ = 4,
- ENA_ADMIN_EXTRA_PROPERTIES_STRINGS = 5,
- ENA_ADMIN_EXTRA_PROPERTIES_FLAGS = 6,
ENA_ADMIN_MAX_QUEUES_EXT = 7,
ENA_ADMIN_RSS_HASH_FUNCTION = 10,
ENA_ADMIN_STATELESS_OFFLOAD_CONFIG = 11,
@@ -599,14 +595,6 @@ struct ena_admin_set_feature_mtu_desc {
u32 mtu;
};
-struct ena_admin_get_extra_properties_strings_desc {
- u32 count;
-};
-
-struct ena_admin_get_extra_properties_flags_desc {
- u32 flags;
-};
-
struct ena_admin_set_feature_host_attr_desc {
/* host OS info base address in OS memory. host info is 4KB of
* physically contiguous
@@ -926,10 +914,6 @@ struct ena_admin_get_feat_resp {
struct ena_admin_feature_intr_moder_desc intr_moderation;
struct ena_admin_ena_hw_hints hw_hints;
-
- struct ena_admin_get_extra_properties_strings_desc extra_properties_strings;
-
- struct ena_admin_get_extra_properties_flags_desc extra_properties_flags;
} u;
};
diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
index 56781609c3af..911a2e7a375a 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_com.c
@@ -1896,62 +1896,6 @@ int ena_com_get_link_params(struct ena_com_dev *ena_dev,
return ena_com_get_feature(ena_dev, resp, ENA_ADMIN_LINK_CONFIG, 0);
}
-int ena_com_extra_properties_strings_init(struct ena_com_dev *ena_dev)
-{
- struct ena_admin_get_feat_resp resp;
- struct ena_extra_properties_strings *extra_properties_strings =
- &ena_dev->extra_properties_strings;
- u32 rc;
-
- extra_properties_strings->size = ENA_ADMIN_EXTRA_PROPERTIES_COUNT *
- ENA_ADMIN_EXTRA_PROPERTIES_STRING_LEN;
-
- extra_properties_strings->virt_addr =
- dma_alloc_coherent(ena_dev->dmadev,
- extra_properties_strings->size,
- &extra_properties_strings->dma_addr,
- GFP_KERNEL);
- if (unlikely(!extra_properties_strings->virt_addr)) {
- pr_err("Failed to allocate extra properties strings\n");
- return 0;
- }
-
- rc = ena_com_get_feature_ex(ena_dev, &resp,
- ENA_ADMIN_EXTRA_PROPERTIES_STRINGS,
- extra_properties_strings->dma_addr,
- extra_properties_strings->size, 0);
- if (rc) {
- pr_debug("Failed to get extra properties strings\n");
- goto err;
- }
-
- return resp.u.extra_properties_strings.count;
-err:
- ena_com_delete_extra_properties_strings(ena_dev);
- return 0;
-}
-
-void ena_com_delete_extra_properties_strings(struct ena_com_dev *ena_dev)
-{
- struct ena_extra_properties_strings *extra_properties_strings =
- &ena_dev->extra_properties_strings;
-
- if (extra_properties_strings->virt_addr) {
- dma_free_coherent(ena_dev->dmadev,
- extra_properties_strings->size,
- extra_properties_strings->virt_addr,
- extra_properties_strings->dma_addr);
- extra_properties_strings->virt_addr = NULL;
- }
-}
-
-int ena_com_get_extra_properties_flags(struct ena_com_dev *ena_dev,
- struct ena_admin_get_feat_resp *resp)
-{
- return ena_com_get_feature(ena_dev, resp,
- ENA_ADMIN_EXTRA_PROPERTIES_FLAGS, 0);
-}
-
int ena_com_get_dev_attr_feat(struct ena_com_dev *ena_dev,
struct ena_com_dev_get_features_ctx *get_feat_ctx)
{
diff --git a/drivers/net/ethernet/amazon/ena/ena_com.h b/drivers/net/ethernet/amazon/ena/ena_com.h
index 4700d92a317b..0d3664fe260d 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.h
+++ b/drivers/net/ethernet/amazon/ena/ena_com.h
@@ -352,12 +352,6 @@ struct ena_host_attribute {
dma_addr_t host_info_dma_addr;
};
-struct ena_extra_properties_strings {
- u8 *virt_addr;
- dma_addr_t dma_addr;
- u32 size;
-};
-
/* Each ena_dev is a PCI function. */
struct ena_com_dev {
struct ena_com_admin_queue admin_queue;
@@ -386,7 +380,6 @@ struct ena_com_dev {
struct ena_intr_moder_entry *intr_moder_tbl;
struct ena_com_llq_info llq_info;
- struct ena_extra_properties_strings extra_properties_strings;
};
struct ena_com_dev_get_features_ctx {
@@ -620,31 +613,6 @@ int ena_com_validate_version(struct ena_com_dev *ena_dev);
int ena_com_get_link_params(struct ena_com_dev *ena_dev,
struct ena_admin_get_feat_resp *resp);
-/* ena_com_extra_properties_strings_init - Initialize the extra properties strings buffer.
- * @ena_dev: ENA communication layer struct
- *
- * Initialize the extra properties strings buffer.
- */
-int ena_com_extra_properties_strings_init(struct ena_com_dev *ena_dev);
-
-/* ena_com_delete_extra_properties_strings - Free the extra properties strings buffer.
- * @ena_dev: ENA communication layer struct
- *
- * Free the allocated extra properties strings buffer.
- */
-void ena_com_delete_extra_properties_strings(struct ena_com_dev *ena_dev);
-
-/* ena_com_get_extra_properties_flags - Retrieve extra properties flags.
- * @ena_dev: ENA communication layer struct
- * @resp: Extra properties flags.
- *
- * Retrieve the extra properties flags.
- *
- * @return - 0 on Success negative value otherwise.
- */
-int ena_com_get_extra_properties_flags(struct ena_com_dev *ena_dev,
- struct ena_admin_get_feat_resp *resp);
-
/* ena_com_get_dma_width - Retrieve physical dma address width the device
* supports.
* @ena_dev: ENA communication layer struct
diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
index b46f069ac0eb..b997c3ce9e2b 100644
--- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
+++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
@@ -198,24 +198,15 @@ static void ena_get_ethtool_stats(struct net_device *netdev,
ena_dev_admin_queue_stats(adapter, &data);
}
-static int get_stats_sset_count(struct ena_adapter *adapter)
-{
- return adapter->num_queues * (ENA_STATS_ARRAY_TX + ENA_STATS_ARRAY_RX)
- + ENA_STATS_ARRAY_GLOBAL + ENA_STATS_ARRAY_ENA_COM;
-}
-
int ena_get_sset_count(struct net_device *netdev, int sset)
{
struct ena_adapter *adapter = netdev_priv(netdev);
- switch (sset) {
- case ETH_SS_STATS:
- return get_stats_sset_count(adapter);
- case ETH_SS_PRIV_FLAGS:
- return adapter->ena_extra_properties_count;
- default:
+ if (sset != ETH_SS_STATS)
return -EOPNOTSUPP;
- }
+
+ return adapter->num_queues * (ENA_STATS_ARRAY_TX + ENA_STATS_ARRAY_RX)
+ + ENA_STATS_ARRAY_GLOBAL + ENA_STATS_ARRAY_ENA_COM;
}
static void ena_queue_strings(struct ena_adapter *adapter, u8 **data)
@@ -257,54 +248,26 @@ static void ena_com_dev_strings(u8 **data)
}
}
-static void get_stats_strings(struct ena_adapter *adapter, u8 *data)
+static void ena_get_strings(struct net_device *netdev, u32 sset, u8 *data)
{
+ struct ena_adapter *adapter = netdev_priv(netdev);
const struct ena_stats *ena_stats;
int i;
+ if (sset != ETH_SS_STATS)
+ return;
+
for (i = 0; i < ENA_STATS_ARRAY_GLOBAL; i++) {
ena_stats = &ena_stats_global_strings[i];
+
memcpy(data, ena_stats->name, ETH_GSTRING_LEN);
data += ETH_GSTRING_LEN;
}
+
ena_queue_strings(adapter, &data);
ena_com_dev_strings(&data);
}
-static void get_private_flags_strings(struct ena_adapter *adapter, u8 *data)
-{
- struct ena_com_dev *ena_dev = adapter->ena_dev;
- u8 *strings = ena_dev->extra_properties_strings.virt_addr;
- int i;
-
- if (unlikely(!strings)) {
- adapter->ena_extra_properties_count = 0;
- return;
- }
-
- for (i = 0; i < adapter->ena_extra_properties_count; i++) {
- strlcpy(data, strings + ENA_ADMIN_EXTRA_PROPERTIES_STRING_LEN * i,
- ETH_GSTRING_LEN);
- data += ETH_GSTRING_LEN;
- }
-}
-
-static void ena_get_strings(struct net_device *netdev, u32 sset, u8 *data)
-{
- struct ena_adapter *adapter = netdev_priv(netdev);
-
- switch (sset) {
- case ETH_SS_STATS:
- get_stats_strings(adapter, data);
- break;
- case ETH_SS_PRIV_FLAGS:
- get_private_flags_strings(adapter, data);
- break;
- default:
- break;
- }
-}
-
static int ena_get_link_ksettings(struct net_device *netdev,
struct ethtool_link_ksettings *link_ksettings)
{
@@ -479,7 +442,6 @@ static void ena_get_drvinfo(struct net_device *dev,
strlcpy(info->version, DRV_MODULE_VERSION, sizeof(info->version));
strlcpy(info->bus_info, pci_name(adapter->pdev),
sizeof(info->bus_info));
- info->n_priv_flags = adapter->ena_extra_properties_count;
}
static void ena_get_ringparam(struct net_device *netdev,
@@ -856,20 +818,6 @@ static int ena_set_tunable(struct net_device *netdev,
return ret;
}
-static u32 ena_get_priv_flags(struct net_device *netdev)
-{
- struct ena_adapter *adapter = netdev_priv(netdev);
- struct ena_com_dev *ena_dev = adapter->ena_dev;
- struct ena_admin_get_feat_resp get_resp;
- u32 rc;
-
- rc = ena_com_get_extra_properties_flags(ena_dev, &get_resp);
- if (!rc)
- return get_resp.u.extra_properties_flags.flags;
-
- return 0;
-}
-
static const struct ethtool_ops ena_ethtool_ops = {
.get_link_ksettings = ena_get_link_ksettings,
.get_drvinfo = ena_get_drvinfo,
@@ -892,7 +840,6 @@ static const struct ethtool_ops ena_ethtool_ops = {
.get_channels = ena_get_channels,
.get_tunable = ena_get_tunable,
.set_tunable = ena_set_tunable,
- .get_priv_flags = ena_get_priv_flags,
};
void ena_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 20ec8ff03aaf..664e3ed97ea9 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -2472,14 +2472,6 @@ static void ena_config_debug_area(struct ena_adapter *adapter)
ena_com_delete_debug_area(adapter->ena_dev);
}
-static void ena_extra_properties_strings_destroy(struct net_device *netdev)
-{
- struct ena_adapter *adapter = netdev_priv(netdev);
-
- ena_com_delete_extra_properties_strings(adapter->ena_dev);
- adapter->ena_extra_properties_count = 0;
-}
-
static void ena_get_stats64(struct net_device *netdev,
struct rtnl_link_stats64 *stats)
{
@@ -3578,9 +3570,6 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
ena_config_debug_area(adapter);
- adapter->ena_extra_properties_count =
- ena_com_extra_properties_strings_init(ena_dev);
-
memcpy(adapter->netdev->perm_addr, adapter->mac_addr, netdev->addr_len);
netif_carrier_off(netdev);
@@ -3620,7 +3609,6 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
return 0;
err_rss:
- ena_extra_properties_strings_destroy(netdev);
ena_com_delete_debug_area(ena_dev);
ena_com_rss_destroy(ena_dev);
err_free_msix:
@@ -3687,8 +3675,6 @@ static void ena_remove(struct pci_dev *pdev)
ena_com_delete_host_info(ena_dev);
- ena_extra_properties_strings_destroy(netdev);
-
ena_release_bars(ena_dev, pdev);
pci_disable_device(pdev);
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h
index f2b6e2e0504d..efbcffd22215 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.h
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h
@@ -378,8 +378,6 @@ struct ena_adapter {
u32 last_monitored_tx_qid;
enum ena_regs_reset_reason_types reset_reason;
-
- u8 ena_extra_properties_count;
};
void ena_set_ethtool_ops(struct net_device *netdev);
--
2.21.0
^ permalink raw reply related
* [PATCH bpf] tools: bpftool: use correct argument in cgroup errors
From: Jakub Kicinski @ 2019-06-25 16:56 UTC (permalink / raw)
To: alexei.starovoitov, daniel
Cc: netdev, bpf, oss-drivers, guro, sdf, Jakub Kicinski,
Quentin Monnet
cgroup code tries to use argv[0] as the cgroup path,
but if it fails uses argv[1] to report errors.
Fixes: 5ccda64d38cc ("bpftool: implement cgroup bpf operations")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/bpf/bpftool/cgroup.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index 73ec8ea33fb4..a13fb7265d1a 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -168,7 +168,7 @@ static int do_show(int argc, char **argv)
cgroup_fd = open(argv[0], O_RDONLY);
if (cgroup_fd < 0) {
- p_err("can't open cgroup %s", argv[1]);
+ p_err("can't open cgroup %s", argv[0]);
goto exit;
}
@@ -356,7 +356,7 @@ static int do_attach(int argc, char **argv)
cgroup_fd = open(argv[0], O_RDONLY);
if (cgroup_fd < 0) {
- p_err("can't open cgroup %s", argv[1]);
+ p_err("can't open cgroup %s", argv[0]);
goto exit;
}
@@ -414,7 +414,7 @@ static int do_detach(int argc, char **argv)
cgroup_fd = open(argv[0], O_RDONLY);
if (cgroup_fd < 0) {
- p_err("can't open cgroup %s", argv[1]);
+ p_err("can't open cgroup %s", argv[0]);
goto exit;
}
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v2 net-next] ipv6: Convert gateway validation to use fib6_info
From: Wei Wang @ 2019-06-25 16:54 UTC (permalink / raw)
To: David Ahern
Cc: David S . Miller, Linux Kernel Network Developers,
Martin KaFai Lau, David Ahern
In-Reply-To: <20190624204451.10929-1-dsahern@kernel.org>
On Mon, Jun 24, 2019 at 1:44 PM David Ahern <dsahern@kernel.org> wrote:
>
> From: David Ahern <dsahern@gmail.com>
>
> Gateway validation does not need a dst_entry, it only needs the fib
> entry to validate the gateway resolution and egress device. So,
> convert ip6_nh_lookup_table from ip6_pol_route to fib6_table_lookup
> and ip6_route_check_nh to use fib6_lookup over rt6_lookup.
>
> ip6_pol_route is a call to fib6_table_lookup and if successful a call
> to fib6_select_path. From there the exception cache is searched for an
> entry or a dst_entry is created to return to the caller. The exception
> entry is not relevant for gateway validation, so what matters are the
> calls to fib6_table_lookup and then fib6_select_path.
>
> Similarly, rt6_lookup can be replaced with a call to fib6_lookup with
> RT6_LOOKUP_F_IFACE set in flags. Again, the exception cache search is
> not relevant, only the lookup with path selection. The primary difference
> in the lookup paths is the use of rt6_select with fib6_lookup versus
> rt6_device_match with rt6_lookup. When you remove complexities in the
> rt6_select path, e.g.,
> 1. saddr is not set for gateway validation, so RT6_LOOKUP_F_HAS_SADDR
> is not relevant
> 2. rt6_check_neigh is not called so that removes the RT6_NUD_FAIL_DO_RR
> return and round-robin logic.
>
> the code paths are believed to be equivalent for the given use case -
> validate the gateway and optionally given the device. Furthermore, it
> aligns the validation with onlink code path and the lookup path actually
> used for rx and tx.
>
> Adjust the users, ip6_route_check_nh_onlink and ip6_route_check_nh to
> handle a fib6_info vs a rt6_info when performing validation checks.
>
> Existing selftests fib-onlink-tests.sh and fib_tests.sh are used to
> verify the changes.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
Reviewed-by: Wei Wang <weiwan@google.com>
> ---
> v2
> - use in6_dev_get versus __in6_dev_get + in6_dev_hold (comment from Wei)
> - updated commit message
>
> net/ipv6/route.c | 118 ++++++++++++++++++++++++++-----------------------------
> 1 file changed, 56 insertions(+), 62 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index be5e65c97652..5fe0fd6f2909 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -3144,10 +3144,9 @@ static int ip6_dst_gc(struct dst_ops *ops)
> return entries > rt_max_size;
> }
>
> -static struct rt6_info *ip6_nh_lookup_table(struct net *net,
> - struct fib6_config *cfg,
> - const struct in6_addr *gw_addr,
> - u32 tbid, int flags)
> +static int ip6_nh_lookup_table(struct net *net, struct fib6_config *cfg,
> + const struct in6_addr *gw_addr, u32 tbid,
> + int flags, struct fib6_result *res)
> {
> struct flowi6 fl6 = {
> .flowi6_oif = cfg->fc_ifindex,
> @@ -3155,25 +3154,23 @@ static struct rt6_info *ip6_nh_lookup_table(struct net *net,
> .saddr = cfg->fc_prefsrc,
> };
> struct fib6_table *table;
> - struct rt6_info *rt;
> + int err;
>
> table = fib6_get_table(net, tbid);
> if (!table)
> - return NULL;
> + return -EINVAL;
>
> if (!ipv6_addr_any(&cfg->fc_prefsrc))
> flags |= RT6_LOOKUP_F_HAS_SADDR;
>
> flags |= RT6_LOOKUP_F_IGNORE_LINKSTATE;
> - rt = ip6_pol_route(net, table, cfg->fc_ifindex, &fl6, NULL, flags);
>
> - /* if table lookup failed, fall back to full lookup */
> - if (rt == net->ipv6.ip6_null_entry) {
> - ip6_rt_put(rt);
> - rt = NULL;
> - }
> + err = fib6_table_lookup(net, table, cfg->fc_ifindex, &fl6, res, flags);
> + if (!err && res->f6i != net->ipv6.fib6_null_entry)
> + fib6_select_path(net, res, &fl6, cfg->fc_ifindex,
> + cfg->fc_ifindex != 0, NULL, flags);
>
> - return rt;
> + return err;
> }
>
> static int ip6_route_check_nh_onlink(struct net *net,
> @@ -3181,29 +3178,19 @@ static int ip6_route_check_nh_onlink(struct net *net,
> const struct net_device *dev,
> struct netlink_ext_ack *extack)
> {
> - u32 tbid = l3mdev_fib_table(dev) ? : RT_TABLE_MAIN;
> + u32 tbid = l3mdev_fib_table_rcu(dev) ? : RT_TABLE_MAIN;
> const struct in6_addr *gw_addr = &cfg->fc_gateway;
> - u32 flags = RTF_LOCAL | RTF_ANYCAST | RTF_REJECT;
> - struct fib6_info *from;
> - struct rt6_info *grt;
> + struct fib6_result res = {};
> int err;
>
> - err = 0;
> - grt = ip6_nh_lookup_table(net, cfg, gw_addr, tbid, 0);
> - if (grt) {
> - rcu_read_lock();
> - from = rcu_dereference(grt->from);
> - if (!grt->dst.error &&
> - /* ignore match if it is the default route */
> - from && !ipv6_addr_any(&from->fib6_dst.addr) &&
> - (grt->rt6i_flags & flags || dev != grt->dst.dev)) {
> - NL_SET_ERR_MSG(extack,
> - "Nexthop has invalid gateway or device mismatch");
> - err = -EINVAL;
> - }
> - rcu_read_unlock();
> -
> - ip6_rt_put(grt);
> + err = ip6_nh_lookup_table(net, cfg, gw_addr, tbid, 0, &res);
> + if (!err && !(res.fib6_flags & RTF_REJECT) &&
> + /* ignore match if it is the default route */
> + !ipv6_addr_any(&res.f6i->fib6_dst.addr) &&
> + (res.fib6_type != RTN_UNICAST || dev != res.nh->fib_nh_dev)) {
> + NL_SET_ERR_MSG(extack,
> + "Nexthop has invalid gateway or device mismatch");
> + err = -EINVAL;
> }
>
> return err;
> @@ -3216,47 +3203,50 @@ static int ip6_route_check_nh(struct net *net,
> {
> const struct in6_addr *gw_addr = &cfg->fc_gateway;
> struct net_device *dev = _dev ? *_dev : NULL;
> - struct rt6_info *grt = NULL;
> + int flags = RT6_LOOKUP_F_IFACE;
> + struct fib6_result res = {};
> int err = -EHOSTUNREACH;
>
> if (cfg->fc_table) {
> - int flags = RT6_LOOKUP_F_IFACE;
> -
> - grt = ip6_nh_lookup_table(net, cfg, gw_addr,
> - cfg->fc_table, flags);
> - if (grt) {
> - if (grt->rt6i_flags & RTF_GATEWAY ||
> - (dev && dev != grt->dst.dev)) {
> - ip6_rt_put(grt);
> - grt = NULL;
> - }
> - }
> + err = ip6_nh_lookup_table(net, cfg, gw_addr,
> + cfg->fc_table, flags, &res);
> + /* gw_addr can not require a gateway or resolve to a reject
> + * route. If a device is given, it must match the result.
> + */
> + if (err || res.fib6_flags & RTF_REJECT ||
> + res.nh->fib_nh_gw_family ||
> + (dev && dev != res.nh->fib_nh_dev))
> + err = -EHOSTUNREACH;
> }
>
> - if (!grt)
> - grt = rt6_lookup(net, gw_addr, NULL, cfg->fc_ifindex, NULL, 1);
> + if (err < 0) {
> + struct flowi6 fl6 = {
> + .flowi6_oif = cfg->fc_ifindex,
> + .daddr = *gw_addr,
> + };
>
> - if (!grt)
> - goto out;
> + err = fib6_lookup(net, cfg->fc_ifindex, &fl6, &res, flags);
> + if (err || res.fib6_flags & RTF_REJECT ||
> + res.nh->fib_nh_gw_family)
> + err = -EHOSTUNREACH;
> +
> + if (err)
> + return err;
> +
> + fib6_select_path(net, &res, &fl6, cfg->fc_ifindex,
> + cfg->fc_ifindex != 0, NULL, flags);
> + }
>
> + err = 0;
> if (dev) {
> - if (dev != grt->dst.dev) {
> - ip6_rt_put(grt);
> - goto out;
> - }
> + if (dev != res.nh->fib_nh_dev)
> + err = -EHOSTUNREACH;
> } else {
> - *_dev = dev = grt->dst.dev;
> - *idev = grt->rt6i_idev;
> + *_dev = dev = res.nh->fib_nh_dev;
> dev_hold(dev);
> - in6_dev_hold(grt->rt6i_idev);
> + *idev = in6_dev_get(dev);
> }
>
> - if (!(grt->rt6i_flags & RTF_GATEWAY))
> - err = 0;
> -
> - ip6_rt_put(grt);
> -
> -out:
> return err;
> }
>
> @@ -3297,11 +3287,15 @@ static int ip6_validate_gw(struct net *net, struct fib6_config *cfg,
> goto out;
> }
>
> + rcu_read_lock();
> +
> if (cfg->fc_flags & RTNH_F_ONLINK)
> err = ip6_route_check_nh_onlink(net, cfg, dev, extack);
> else
> err = ip6_route_check_nh(net, cfg, _dev, idev);
>
> + rcu_read_unlock();
> +
> if (err)
> goto out;
> }
> --
> 2.11.0
>
^ permalink raw reply
* Re: 4.19: Traced deadlock during xfrm_user module load
From: Florian Westphal @ 2019-06-25 16:53 UTC (permalink / raw)
To: Thomas Jarosch; +Cc: netdev, netfilter-devel, Juliana Rodrigueiro
In-Reply-To: <20190625155509.pgcxwgclqx3lfxxr@intra2net.com>
Thomas Jarosch <thomas.jarosch@intra2net.com> wrote:
> we're in the process of upgrading to kernel 4.19 and hit
> a very rare lockup on boot during "xfrm_user" module load.
> The tested kernel was 4.19.55.
>
> When the strongswan IPsec service starts, it loads the xfrm_user module.
> -> modprobe hangs forever.
>
> Also network services like ssh or apache stop responding,
> ICMP ping still works.
>
> By chance we had magic sysRq enabled and were able to get some meaningful stack
> traces. We've rebuilt the kernel with LOCKDEP + DEBUG_INFO + DEBUG_INFO_REDUCED,
> but so far failed to reproduce the issue even when hammering the suspected
> deadlock case. Though it's just hammering it for a few hours yet.
>
> Preliminary analysis:
>
> "modprobe xfrm_user":
> xfrm_user_init()
> register_pernet_subsys()
> -> grab pernet_ops_rwsem
> ..
> netlink_table_grab()
> calls schedule() as "nl_table_users" is non-zero
>
>
> conntrack netlink related program "info_iponline" does this in parallel:
> netlink_bind()
> netlink_lock_table() -> increases "nl_table_users"
> nfnetlink_bind()
> # does not unlock the table as it's locked by netlink_bind()
> __request_module()
> call_usermodehelper_exec()
>
>
> "modprobe nf_conntrack_netlink" runs and inits nf_conntrack_netlink:
> ctnetlink_init()
> register_pernet_subsys()
> -> blocks on "pernet_ops_rwsem" thanks to xfrm_user module
> -> schedule()
> -> deadlock forever
>
Thanks for this detailed analysis.
In this specific case I think this is enough:
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 92077d459109..61ba92415480 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -578,7 +578,8 @@ static int nfnetlink_bind(struct net *net, int group)
ss = nfnetlink_get_subsys(type << 8);
rcu_read_unlock();
if (!ss)
- request_module("nfnetlink-subsys-%d", type);
+ request_module_nowait("nfnetlink-subsys-%d", type);
return 0;
}
#endif
^ permalink raw reply related
* [PATCH bpf] bpf: fix BPF_ALU32 | BPF_ARSH on BE arches
From: Jiong Wang @ 2019-06-25 16:41 UTC (permalink / raw)
To: alexei.starovoitov, daniel
Cc: yauheni.kaliuta, bpf, netdev, oss-drivers, Jiong Wang
Yauheni reported the following code do not work correctly on BE arches:
ALU_ARSH_X:
DST = (u64) (u32) ((*(s32 *) &DST) >> SRC);
CONT;
ALU_ARSH_K:
DST = (u64) (u32) ((*(s32 *) &DST) >> IMM);
CONT;
and are causing failure of test_verifier test 'arsh32 on imm 2' on BE
arches.
The code is taking address and interpreting memory directly, so is not
endianness neutral. We should instead perform standard C type casting on
the variable. A u64 to s32 conversion will drop the high 32-bit and reserve
the low 32-bit as signed integer, this is all we want.
Fixes: 2dc6b100f928 ("bpf: interpreter support BPF_ALU | BPF_ARSH")
Reported-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
---
kernel/bpf/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 080e2bb..f2148db 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1364,10 +1364,10 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u64 *stack)
insn++;
CONT;
ALU_ARSH_X:
- DST = (u64) (u32) ((*(s32 *) &DST) >> SRC);
+ DST = (u64) (u32) (((s32) DST) >> SRC);
CONT;
ALU_ARSH_K:
- DST = (u64) (u32) ((*(s32 *) &DST) >> IMM);
+ DST = (u64) (u32) (((s32) DST) >> IMM);
CONT;
ALU64_ARSH_X:
(*(s64 *) &DST) >>= SRC;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net] net/sched: flower: fix infinite loop in fl_walk()
From: Davide Caratti @ 2019-06-25 16:23 UTC (permalink / raw)
To: Cong Wang
Cc: Vlad Buslov, David S. Miller, Linux Kernel Network Developers,
Lucas Bates
In-Reply-To: <6650f0da68982ffa5bb71a773c5a3d588bd972c4.camel@redhat.com>
On Tue, 2019-06-25 at 17:47 +0200, Davide Caratti wrote:
> On Thu, 2019-06-20 at 10:33 -0700, Cong Wang wrote:
> > On Thu, Jun 20, 2019 at 5:52 AM Davide Caratti <dcaratti@redhat.com> wrote:
> > > hello Cong, thanks for reading.
> > >
> > > On Wed, 2019-06-19 at 15:04 -0700, Cong Wang wrote:
> > > > On Wed, Jun 19, 2019 at 2:10 PM Davide Caratti <dcaratti@redhat.com> wrote:
> > > > > on some CPUs (e.g. i686), tcf_walker.cookie has the same size as the IDR.
> > > > > In this situation, the following script:
> > > > >
> > > > > # tc filter add dev eth0 ingress handle 0xffffffff flower action ok
> > > > > # tc filter show dev eth0 ingress
> > > > >
> > > > > results in an infinite loop.
>
> So, when the radix tree contains one slot with index equal to ULONG_MAX,
> whatever can be the value of 'id',
oops, this phrase is of course wrong. the value of 'id' matters to
determine the condition of the if().
> the condition in that if() will always
> be false (and the function will keep returning non-NULL, hence the
> infinite loop).
what I wanted to say is, when the radix tree contains a single slot with
index equal to ULONG_MAX, whatever value I put in 'id' the function will
always return a pointer to that slot.
--
davide
^ permalink raw reply
* Re: [PATCH v3 net] af_packet: Block execution of tasks waiting for transmit to complete in AF_PACKET
From: Neil Horman @ 2019-06-25 16:20 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, Matteo Croce, David S. Miller
In-Reply-To: <CAF=yD-KEZBds_SRDFnOjqvidW30E=NG-2X=hBdcGx_--PAmjew@mail.gmail.com>
On Tue, Jun 25, 2019 at 09:37:17AM -0400, Willem de Bruijn wrote:
> On Tue, Jun 25, 2019 at 7:03 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Mon, Jun 24, 2019 at 06:15:29PM -0400, Willem de Bruijn wrote:
> > > > > > + if (need_wait && !packet_next_frame(po, &po->tx_ring, TP_STATUS_SEND_REQUEST)) {
> > > > > > + po->wait_on_complete = 1;
> > > > > > + timeo = sock_sndtimeo(&po->sk, msg->msg_flags & MSG_DONTWAIT);
> > > > >
> > > > > This resets timeout on every loop. should only set above the loop once.
> > > > >
> > > > I explained exactly why I did that in the change log. Its because I reuse the
> > > > timeout variable to get the return value of the wait_for_complete call.
> > > > Otherwise I need to add additional data to the stack, which I don't want to do.
> > > > Sock_sndtimeo is an inline function and really doesn't add any overhead to this
> > > > path, so I see no reason not to reuse the variable.
> > >
> > > The issue isn't the reuse. It is that timeo is reset to sk_sndtimeo
> > > each time. Whereas wait_for_common and its variants return the
> > > number of jiffies left in case the loop needs to sleep again later.
> > >
> > > Reading sock_sndtimeo once and passing it to wait_.. repeatedly is a
> > > common pattern across the stack.
> > >
> > But those patterns are unique to those situations. For instance, in
> > tcp_sendmsg_locked, we aquire the value of the socket timeout, and use that to
> > wait for the entire message send operation to complete, which consists of
> > potentially several blocking operations (waiting for the tcp connection to be
> > established, waiting for socket memory, etc). In that situation we want to wait
> > for all of those operations to complete to send a single message, and fail if
> > they exceed the timeout in aggregate. The semantics are different with
> > AF_PACKET. In this use case, the message is in effect empty, and just used to
> > pass some control information. tpacket_snd, sends as many frames from the
> > memory mapped buffer as possible, and on each iteration we want to wait for the
> > specified timeout for those frames to complete sending. I think resetting the
> > timeout on each wait instance is the right way to go here.
>
> I disagree. If a SO_SNDTIMEO of a given time is set, the thread should
> not wait beyond that. Else what is the point of passing a specific
> duration in the syscall?
>
I can appreciate that, but you said yourself that you wanted to minimize this
change. The current behavior of AF_PACKET is to:
a) check for their being no more packets ready to send
b) if (a) is false we schedule() (nominally allowing other tasks to run)
c) when (b) is complete, check for additional frames to send, and exit if there
are not, otherwise, continue sending and waiting
In that model, a single task calling sendmsg can run in the kernel indefinately,
if userspace continues to fill the memory mapped buffer
Given that model, resetting the timeout on each call to wait_for_completion
keeps that behavior. In fact, if we allow the timeout value to get continuously
decremented, it will be possible for a call to sendmsg in this protocol to
_always_ return -ETIMEDOUT (if we keep the buffer full in userspace long enough,
then the sending task will eventually decrement timeo to zero, and force an
-ETIMEDOUT call).
I'm convinced that resetting the timeout here is the right way to go.
> Btw, we can always drop the timeo and go back to unconditional (bar
> signals) waiting.
>
We could, but it would be nice to implement the timeout feature here if
possible.
> >
> > > > > > @@ -2728,6 +2755,11 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> > > > > > err = net_xmit_errno(err);
> > > > > > if (err && __packet_get_status(po, ph) ==
> > > > > > TP_STATUS_AVAILABLE) {
> > > > > > + /* re-init completion queue to avoid subsequent fallthrough
> > > > > > + * on a future thread calling wait_on_complete_interruptible_timeout
> > > > > > + */
> > > > > > + po->wait_on_complete = 0;
> > > > >
> > > > > If setting where sleeping, no need for resetting if a failure happens
> > > > > between those blocks.
> > > > >
> > > > > > + init_completion(&po->skb_completion);
> > > > >
> > > > > no need to reinit between each use?
> > > > >
> > > > I explained exactly why I did this in the comment above. We have to set
> > > > wait_for_complete prior to calling transmit, so as to ensure that we call
> > > > wait_for_completion before we exit the loop. However, in this error case, we
> > > > exit the loop prior to calling wait_for_complete, so we need to reset the
> > > > completion variable and the wait_for_complete flag. Otherwise we will be in a
> > > > case where, on the next entrace to this loop we will have a completion variable
> > > > with completion->done > 0, meaning the next wait will be a fall through case,
> > > > which we don't want.
> > >
> > > By moving back to the point where schedule() is called, hopefully this
> > > complexity automatically goes away. Same as my comment to the line
> > > immediately above.
> > >
> > Its going to change what the complexity is, actually. I was looking at this
> > last night, and I realized that your assertion that we could remove
> > packet_next_frame came at a cost. This is because we need to determine if we
> > have to wait before we call po->xmit, but we need to actually do the wait after
> > po->xmit
>
> Why? The existing method using schedule() doesn't.
>
Because the existing method using schedule doesn't have to rely on an
asynchronous event to wake the sending task back up. Specifically, we need to
set some state that indicates tpacket_destruct_skb should call complete, and
since tpacket_destruct_skb is called asynchronously in a separate execution
context, we need to ensure there is no race condition whereby we execute
tpacket_destruct_skb before we set the state that we also use to determine if we
should call wait_for_complete. ie:
1) tpacket_send_skb calls po->xmit
2) tpacket_send_skb loops around and checks to see if there are more packets to
send. If not and if need_wait is set we call wait_for_complete
If tpacket_destruct_skb is called after (2), we're fine. But if its called
between (1) and (2), then tpacket_destruct_skb won't call complete (because
wait_on_complete isn't set yet), causing a hang.
Because you wanted to remove packet_next_frame, we have no way to determine if
we're done sending frames prior to calling po->xmit, which is the point past
which tpacket_destruct_skb might be called, so now I have to find an alternate
state condition to key off of.
> Can we not just loop while sending and sleep immediately when
> __packet_get_status returns !TP_STATUS_AVAILABLE?
>
> I don't understand the need to probe the next packet to send instead
> of the current.
>
See above, we can definately check the return of ph at the top of the loop, and
sleep if its NULL, but in so doing we cant use po->wait_on_complete as a gating
variable because we've already called po->xmit. Once we call that function, if
we haven't already set po->wait_on_complete to true, its possible
tpacket_destruct_skb will already have been called before we get to the point
where we check wait_for_completion. That means we will wait on a completion
variable that never gets completed, so I need to find a new way to track weather
or not we are waiting. Its entirely doable, I'm sure, its just not as straight
forward as your making it out to be.
> This seems to be the crux of the disagreement. My guess is that it has
> to do with setting wait_on_complete, but I don't see the problem. It
> can be set immediately before going to sleep.
>
The reason has to do with the fact that
tpacket_destruct_skb can be called from ksoftirq context (i.e. it will be called
asynchrnously, not from the thread running in tpacket_snd). Condsider this
flow, assuming we do as you suggest, and just set po->wait_on_complete=1
immediately prior to calling wait_for_complete_interruptible_timeout:
1) tpacket_snd gets a frame, builds the skb for transmission
2) tpakcket_snd calls po->xmit(skb), sending the frame to the driver
3) tpacket_snd, iterates through back to the top of the loop, and determines
that we need to wait for the previous packet to complete
4) The driver gets a tx completion interrupt, interrupting the cpu on which we
are executing, and calls kfree_skb_irq on the skb we just transmitted
5) the ksoftirq runs on the cpu, calling kfree_skb on our skb
6) (5) calls skb->destruct (which is tpakcet_skb_destruct)
7) tpacket_skb_destruct executes, sees that po->wait_on_completion is zero, and
skips calling complete()
8) the thread running tpacket_snd gets scheduled back on the cpu and now sets
po->wait_on_complete, then calls wait_for_completion_interruptible_timeout, but
since the skb we are waiting to complete has already been freed, we will never
get the completion notification, and so we will wait for the maximal timeout,
which is absolutely not what we want.
Interestingly (Ironically), that control flow will never happen in the use case
I'm trying to fix here, because its SCHED_FIFO, and will run until such time as
we call wait_for_completion_interuptible_timeout, so in this case we're safe.
But in the nominal case, where the sending task is acturally SCHED_OTHER, the
above can aboslutely happen, and thats very broken.
> I don't meant to draw this out, btw, or add to your workload. If you
> prefer, I can take a stab at my (admittedly hand-wavy) suggestion.
>
No, I have another method in flight that I'm testing now, it removes the
po->wait_on_complete variable entirely, checking instead to make sure that
we've:
a) sent at least one frame
and
b) that we have a positive pending count
and
c) that we need to wait
and
d) that we have no more frames to send
This is why I was saying that your idea can be done, but it trades one form of
complexity for another.
Neil
> > (to ensure that wait_on_complete is set properly when the desructor is
> > called). By moving the wait to the top of the loop and getting rid of
> > packet_next_frame we now have a race condition in which we might call
> > tpacket_destruct_skb with wait_on_complete set to false, causing us to wait for
> > the maximum timeout erroneously. So I'm going to have to find a new way to
> > signal the need to call complete, which I think will introduce a different level
> > of complexity.
>
^ permalink raw reply
* Re: [PATCH net] vxlan: do not destroy fdb if register_netdevice() is failed
From: Taehee Yoo @ 2019-06-25 16:08 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: David Miller, Netdev
In-Reply-To: <CAJieiUjri=-w2PqB9q5fEa=4jqkTWSfK0dUwnT7Cvxdo2sRRzg@mail.gmail.com>
On Tue, 25 Jun 2019 at 13:12, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>
Hi Roopa,
Thank you for the review!
> On Sun, Jun 23, 2019 at 7:18 PM Taehee Yoo <ap420073@gmail.com> wrote:
> >
> > On Mon, 24 Jun 2019 at 03:07, David Miller <davem@davemloft.net> wrote:
> > >
> >
> > Hi David,
> >
> > Thank you for the review!
> >
> > > From: Taehee Yoo <ap420073@gmail.com>
> > > Date: Thu, 20 Jun 2019 20:51:08 +0900
> > >
> > > > __vxlan_dev_create() destroys FDB using specific pointer which indicates
> > > > a fdb when error occurs.
> > > > But that pointer should not be used when register_netdevice() fails because
> > > > register_netdevice() internally destroys fdb when error occurs.
> > > >
> > > > In order to avoid un-registered dev's notification, fdb destroying routine
> > > > checks dev's register status before notification.
> > >
> > > Simply pass do_notify as false in this failure code path of __vxlan_dev_create(),
> > > thank you.
> >
> > Failure path of __vxlan_dev_create() can't handle do_notify in that case
> > because if register_netdevice() fails it internally calls
> > ->ndo_uninit() which is
> > vxlan_uninit().
> > vxlan_uninit() internally calls vxlan_fdb_delete_default() and it callls
> > vxlan_fdb_destroy().
> > do_notify of vxlan_fdb_destroy() in vxlan_fdb_delete_default() is always true.
> > So, failure path of __vxlan_dev_create() doesn't have any opportunity to
> > handle do_notify.
>
>
> I don't see register_netdevice calling ndo_uninit in case of all
> errors. In the case where it does not,
> does your patch leak the fdb entry ?.
>
> Wondering if we should just use vxlan_fdb_delete_default with a notify
> flag to delete the entry if exists.
> Will that help ?
>
> There is another commit that touched this code path:
> commit 6db9246871394b3a136cd52001a0763676563840
>
> Author: Petr Machata <petrm@mellanox.com>
> Date: Tue Dec 18 13:16:00 2018 +0000
> vxlan: Fix error path in __vxlan_dev_create()
I have checked up failure path of register_netdevice().
Yes, this patch leaks fdb entry.
There are 3 failure cases in the register_netdevice().
A. error occurs before calling ->ndo_init().
it doesn't call ->ndo_uninit().
B. error occurs after calling ->ndo_init().
it calls ->ndo_uninit() and dev->reg_state is NETREG_UNINITIALIZED.
C. error occurs after registering netdev. it calls rollback_registered().
rollback_registered() internally calls ->ndo_uninit()
and dev->reg_state is NETREG_UNREGISTERING.
A panic due to these problem could be fixed by using
vxlan_fdb_delete_default() with notify flag.
But notification problem could not be fixed clearly
because of the case C.
I don't have clear solution for the case C.
Please let me know, if you have any good idea for fixing the case C.
Thank you!
^ permalink raw reply
* [net-next 1/1] tipc: eliminate unnecessary skb expansion during retransmission
From: Jon Maloy @ 2019-06-25 16:08 UTC (permalink / raw)
To: davem, netdev
Cc: gordan.mihaljevic, tung.q.nguyen, hoang.h.le, jon.maloy,
canh.d.luu, ying.xue, tipc-discussion
We increase the allocated headroom for the buffer copies to be
retransmitted. This eliminates the need for the lower stack levels
(UDP/IP/L2) to expand the headroom in order to add their own headers.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/link.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/tipc/link.c b/net/tipc/link.c
index af50b53..aa79bf8 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1125,7 +1125,7 @@ static int tipc_link_bc_retrans(struct tipc_link *l, struct tipc_link *r,
continue;
TIPC_SKB_CB(skb)->nxt_retr = jiffies + TIPC_BC_RETR_LIM;
}
- _skb = __pskb_copy(skb, MIN_H_SIZE, GFP_ATOMIC);
+ _skb = __pskb_copy(skb, LL_MAX_HEADER + MIN_H_SIZE, GFP_ATOMIC);
if (!_skb)
return 0;
hdr = buf_msg(_skb);
--
2.1.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox