* [net-next 09/10] net/mlx5e: Expose same physical switch_id for all representors
From: Saeed Mahameed @ 2019-06-28 23:18 UTC (permalink / raw)
To: David S. Miller
Cc: netdev@vger.kernel.org, Paul Blakey, Parav Pandit, Roi Dayan,
Saeed Mahameed
In-Reply-To: <20190628231759.16374-1-saeedm@mellanox.com>
From: Paul Blakey <paulb@mellanox.com>
Report system_image_guid as the E-Switch switch_id, this ensures
that when a NIC contains multiple PCI functions and which
has merged eswitch capability, all representors from
multiple PFs publish same switch_id.
Signed-off-by: Paul Blakey <paulb@mellanox.com>
Reviewed-by: Parav Pandit <parav@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
.../net/ethernet/mellanox/mlx5/core/en_rep.c | 29 ++++++-------------
1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index fce3814bdb2f..330034fcdfc5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -391,30 +391,19 @@ static const struct ethtool_ops mlx5e_uplink_rep_ethtool_ops = {
static int mlx5e_rep_get_port_parent_id(struct net_device *dev,
struct netdev_phys_item_id *ppid)
{
- struct mlx5e_priv *priv = netdev_priv(dev);
- struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
- struct net_device *uplink_upper = NULL;
- struct mlx5e_priv *uplink_priv = NULL;
- struct net_device *uplink_dev;
+ struct mlx5_eswitch *esw;
+ struct mlx5e_priv *priv;
+ u64 parent_id;
+
+ priv = netdev_priv(dev);
+ esw = priv->mdev->priv.eswitch;
if (esw->mode == SRIOV_NONE)
return -EOPNOTSUPP;
- uplink_dev = mlx5_eswitch_uplink_get_proto_dev(esw, REP_ETH);
- if (uplink_dev) {
- uplink_upper = netdev_master_upper_dev_get(uplink_dev);
- uplink_priv = netdev_priv(uplink_dev);
- }
-
- ppid->id_len = ETH_ALEN;
- if (uplink_upper && mlx5_lag_is_sriov(uplink_priv->mdev)) {
- ether_addr_copy(ppid->id, uplink_upper->dev_addr);
- } else {
- struct mlx5e_rep_priv *rpriv = priv->ppriv;
- struct mlx5_eswitch_rep *rep = rpriv->rep;
-
- ether_addr_copy(ppid->id, rep->hw_id);
- }
+ parent_id = mlx5_query_nic_system_image_guid(priv->mdev);
+ ppid->id_len = sizeof(parent_id);
+ memcpy(ppid->id, &parent_id, sizeof(parent_id));
return 0;
}
--
2.21.0
^ permalink raw reply related
* [net-next 08/10] net/mlx5e: Don't refresh TIRs when updating representor SQs
From: Saeed Mahameed @ 2019-06-28 23:18 UTC (permalink / raw)
To: David S. Miller
Cc: netdev@vger.kernel.org, Gavi Teitz, Roi Dayan, Tariq Toukan,
Saeed Mahameed
In-Reply-To: <20190628231759.16374-1-saeedm@mellanox.com>
From: Gavi Teitz <gavi@mellanox.com>
Refreshing TIRs is done in order to update the TIRs with the current
state of SQs in the transport domain, so that the TIRs can filter out
undesired self-loopback packets based on the source SQ of the packet.
Representor TIRs will only receive packets that originate from their
associated vport, due to dedicated steering, and therefore will never
receive self-loopback packets, whose source vport will be the vport of
the E-Switch manager, and therefore not the vport associated with the
representor. As such, it is not necessary to refresh the representors'
TIRs, since self-loopback packets can't reach them.
Since representors only exist in switchdev mode, and there is no
scenario in which a representor will exist in the transport domain
alongside a non-representor, it is not necessary to refresh the
transport domain's TIRs upon changing the state of a representor's
queues. Therefore, do not refresh TIRs upon such a change. Achieve
this by adding an update_rx callback to the mlx5e_profile, which
refreshes TIRs for non-representors and does nothing for representors,
and replace instances of mlx5e_refresh_tirs() upon changing the state
of the queues with update_rx().
Signed-off-by: Gavi Teitz <gavi@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en.h | 2 ++
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 10 ++++++++--
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 7 +++++++
drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c | 3 ++-
.../net/ethernet/mellanox/mlx5/core/ipoib/ipoib_vlan.c | 3 ++-
5 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 3fe3fe128256..3281ace5e126 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -763,6 +763,7 @@ struct mlx5e_profile {
void (*cleanup_tx)(struct mlx5e_priv *priv);
void (*enable)(struct mlx5e_priv *priv);
void (*disable)(struct mlx5e_priv *priv);
+ int (*update_rx)(struct mlx5e_priv *priv);
void (*update_stats)(struct mlx5e_priv *priv);
void (*update_carrier)(struct mlx5e_priv *priv);
struct {
@@ -1034,6 +1035,7 @@ int mlx5e_create_tis(struct mlx5_core_dev *mdev, int tc,
void mlx5e_destroy_tis(struct mlx5_core_dev *mdev, u32 tisn);
int mlx5e_create_tises(struct mlx5e_priv *priv);
+int mlx5e_update_nic_rx(struct mlx5e_priv *priv);
void mlx5e_update_carrier(struct mlx5e_priv *priv);
int mlx5e_close(struct net_device *netdev);
int mlx5e_open(struct net_device *netdev);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 3df663d6e4d8..1085040675ae 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -2845,7 +2845,7 @@ static void mlx5e_switch_priv_channels(struct mlx5e_priv *priv,
if (hw_modify)
hw_modify(priv);
- mlx5e_refresh_tirs(priv, false);
+ priv->profile->update_rx(priv);
mlx5e_activate_priv_channels(priv);
/* return carrier back if needed */
@@ -2892,7 +2892,7 @@ int mlx5e_open_locked(struct net_device *netdev)
if (err)
goto err_clear_state_opened_flag;
- mlx5e_refresh_tirs(priv, false);
+ priv->profile->update_rx(priv);
mlx5e_activate_priv_channels(priv);
if (priv->profile->update_carrier)
priv->profile->update_carrier(priv);
@@ -4928,6 +4928,11 @@ static void mlx5e_nic_disable(struct mlx5e_priv *priv)
mlx5_lag_remove(mdev);
}
+int mlx5e_update_nic_rx(struct mlx5e_priv *priv)
+{
+ return mlx5e_refresh_tirs(priv, false);
+}
+
static const struct mlx5e_profile mlx5e_nic_profile = {
.init = mlx5e_nic_init,
.cleanup = mlx5e_nic_cleanup,
@@ -4937,6 +4942,7 @@ static const struct mlx5e_profile mlx5e_nic_profile = {
.cleanup_tx = mlx5e_cleanup_nic_tx,
.enable = mlx5e_nic_enable,
.disable = mlx5e_nic_disable,
+ .update_rx = mlx5e_update_nic_rx,
.update_stats = mlx5e_update_ndo_stats,
.update_carrier = mlx5e_update_carrier,
.rx_handlers.handle_rx_cqe = mlx5e_handle_rx_cqe,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index dee2fbbf3c12..fce3814bdb2f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -1638,6 +1638,11 @@ static void mlx5e_rep_enable(struct mlx5e_priv *priv)
mlx5e_set_netdev_mtu_boundaries(priv);
}
+static int mlx5e_update_rep_rx(struct mlx5e_priv *priv)
+{
+ return 0;
+}
+
static int uplink_rep_async_event(struct notifier_block *nb, unsigned long event, void *data)
{
struct mlx5e_priv *priv = container_of(nb, struct mlx5e_priv, events_nb);
@@ -1713,6 +1718,7 @@ static const struct mlx5e_profile mlx5e_rep_profile = {
.init_tx = mlx5e_init_rep_tx,
.cleanup_tx = mlx5e_cleanup_rep_tx,
.enable = mlx5e_rep_enable,
+ .update_rx = mlx5e_update_rep_rx,
.update_stats = mlx5e_rep_update_hw_counters,
.rx_handlers.handle_rx_cqe = mlx5e_handle_rx_cqe_rep,
.rx_handlers.handle_rx_cqe_mpwqe = mlx5e_handle_rx_cqe_mpwrq,
@@ -1728,6 +1734,7 @@ static const struct mlx5e_profile mlx5e_uplink_rep_profile = {
.cleanup_tx = mlx5e_cleanup_rep_tx,
.enable = mlx5e_uplink_rep_enable,
.disable = mlx5e_uplink_rep_disable,
+ .update_rx = mlx5e_update_rep_rx,
.update_stats = mlx5e_uplink_rep_update_hw_counters,
.update_carrier = mlx5e_update_carrier,
.rx_handlers.handle_rx_cqe = mlx5e_handle_rx_cqe_rep,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
index 9ca492b430d8..e68d124eb625 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
@@ -418,6 +418,7 @@ static const struct mlx5e_profile mlx5i_nic_profile = {
.cleanup_rx = mlx5i_cleanup_rx,
.enable = NULL, /* mlx5i_enable */
.disable = NULL, /* mlx5i_disable */
+ .update_rx = mlx5e_update_nic_rx,
.update_stats = NULL, /* mlx5i_update_stats */
.update_carrier = NULL, /* no HW update in IB link */
.rx_handlers.handle_rx_cqe = mlx5i_handle_rx_cqe,
@@ -526,7 +527,7 @@ static int mlx5i_open(struct net_device *netdev)
if (err)
goto err_remove_fs_underlay_qp;
- mlx5e_refresh_tirs(epriv, false);
+ epriv->profile->update_rx(epriv);
mlx5e_activate_priv_channels(epriv);
mutex_unlock(&epriv->state_lock);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib_vlan.c b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib_vlan.c
index b491b8f5fd6b..e05186ada721 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib_vlan.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib_vlan.c
@@ -221,7 +221,7 @@ static int mlx5i_pkey_open(struct net_device *netdev)
mlx5_core_warn(mdev, "opening child channels failed, %d\n", err);
goto err_clear_state_opened_flag;
}
- mlx5e_refresh_tirs(epriv, false);
+ epriv->profile->update_rx(epriv);
mlx5e_activate_priv_channels(epriv);
mutex_unlock(&epriv->state_lock);
@@ -350,6 +350,7 @@ static const struct mlx5e_profile mlx5i_pkey_nic_profile = {
.cleanup_rx = mlx5i_pkey_cleanup_rx,
.enable = NULL,
.disable = NULL,
+ .update_rx = mlx5e_update_nic_rx,
.update_stats = NULL,
.rx_handlers.handle_rx_cqe = mlx5i_handle_rx_cqe,
.rx_handlers.handle_rx_cqe_mpwqe = NULL, /* Not supported */
--
2.21.0
^ permalink raw reply related
* [net-next 10/10] net/mlx5e: Disallow tc redirect offload cases we don't support
From: Saeed Mahameed @ 2019-06-28 23:18 UTC (permalink / raw)
To: David S. Miller
Cc: netdev@vger.kernel.org, Paul Blakey, Roi Dayan, Saeed Mahameed
In-Reply-To: <20190628231759.16374-1-saeedm@mellanox.com>
From: Paul Blakey <paulb@mellanox.com>
After changing the parent_id to be the same for both NICs of same
the hardware device, netdev_port_same_parent_id now returns true for
more cases (all the lower devices in the hierarchy are on the same
hardware device).
If merged eswitch isn't enabled, these cases aren't supported, so disallow
them.
Signed-off-by: Paul Blakey <paulb@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
.../ethernet/mellanox/mlx5/core/en/tc_tun.c | 4 +++-
.../net/ethernet/mellanox/mlx5/core/en_tc.c | 22 +++++++++++++++----
.../net/ethernet/mellanox/mlx5/core/en_tc.h | 3 +++
3 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
index f5ad531e1749..3739646b653f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
@@ -5,6 +5,7 @@
#include <net/gre.h>
#include <net/geneve.h>
#include "en/tc_tun.h"
+#include "en_tc.h"
struct mlx5e_tc_tunnel *mlx5e_get_tc_tun(struct net_device *tunnel_dev)
{
@@ -47,7 +48,8 @@ static int get_route_and_out_devs(struct mlx5e_priv *priv,
*route_dev = dev;
if (is_vlan_dev(*route_dev))
*out_dev = uplink_dev;
- else if (mlx5e_eswitch_rep(dev))
+ else if (mlx5e_eswitch_rep(dev) &&
+ mlx5e_is_valid_eswitch_fwd_dev(priv, dev))
*out_dev = *route_dev;
else
return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 1453da6ef559..e6b199cd68ea 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2802,6 +2802,16 @@ static int add_vlan_pop_action(struct mlx5e_priv *priv,
return err;
}
+bool mlx5e_is_valid_eswitch_fwd_dev(struct mlx5e_priv *priv,
+ struct net_device *out_dev)
+{
+ if (is_merged_eswitch_dev(priv, out_dev))
+ return true;
+
+ return mlx5e_eswitch_rep(out_dev) &&
+ same_hw_devs(priv, netdev_priv(out_dev));
+}
+
static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
struct flow_action *flow_action,
struct mlx5e_tc_flow *flow,
@@ -2867,9 +2877,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
action |= MLX5_FLOW_CONTEXT_ACTION_FWD_DEST |
MLX5_FLOW_CONTEXT_ACTION_COUNT;
- if (netdev_port_same_parent_id(priv->netdev,
- out_dev) ||
- is_merged_eswitch_dev(priv, out_dev)) {
+ if (netdev_port_same_parent_id(priv->netdev, out_dev)) {
struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
struct net_device *uplink_dev = mlx5_eswitch_uplink_get_proto_dev(esw, REP_ETH);
struct net_device *uplink_upper = netdev_master_upper_dev_get(uplink_dev);
@@ -2886,6 +2894,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
if (err)
return err;
}
+
if (is_vlan_dev(parse_attr->filter_dev)) {
err = add_vlan_pop_action(priv, attr,
&action);
@@ -2893,8 +2902,13 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
return err;
}
- if (!mlx5e_eswitch_rep(out_dev))
+ if (!mlx5e_is_valid_eswitch_fwd_dev(priv, out_dev)) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "devices are not on same switch HW, can't offload forwarding");
+ pr_err("devices %s %s not on same switch HW, can't offload forwarding\n",
+ priv->netdev->name, out_dev->name);
return -EOPNOTSUPP;
+ }
out_priv = netdev_priv(out_dev);
rpriv = out_priv->ppriv;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.h b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.h
index f62e81902d27..8f288cc53cee 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.h
@@ -74,6 +74,9 @@ int mlx5e_tc_num_filters(struct mlx5e_priv *priv, int flags);
void mlx5e_tc_reoffload_flows_work(struct work_struct *work);
+bool mlx5e_is_valid_eswitch_fwd_dev(struct mlx5e_priv *priv,
+ struct net_device *out_dev);
+
#else /* CONFIG_MLX5_ESWITCH */
static inline int mlx5e_tc_nic_init(struct mlx5e_priv *priv) { return 0; }
static inline void mlx5e_tc_nic_cleanup(struct mlx5e_priv *priv) {}
--
2.21.0
^ permalink raw reply related
* Re: [PATCH iproute2-next v4 1/2] ipaddress: correctly print a VF hw address in the IPoIB case
From: David Ahern @ 2019-06-28 23:21 UTC (permalink / raw)
To: Denis Kirjanov, stephen; +Cc: netdev, linux-rdma, dledford, mkubecek
In-Reply-To: <20190628095426.2819-1-dkirjanov@suse.com>
On 6/28/19 3:54 AM, Denis Kirjanov wrote:
> Current code assumes that we print ethernet mac and
> that doesn't work in the IPoIB case with SRIOV-enabled hardware
>
> Before:
> 11: ib1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc pfifo_fast
> state UP mode DEFAULT group default qlen 256
> link/infiniband
> 80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
> 00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff
> vf 0 MAC 14:80:00:00:66:fe, spoof checking off, link-state
> disable,
> trust off, query_rss off
> ...
>
> After:
> 11: ib1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc pfifo_fast
> state UP mode DEFAULT group default qlen 256
> link/infiniband
> 80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
> 00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff
> vf 0 link/infiniband
> 80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
> 00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff, spoof
> checking off, link-state disable, trust off, query_rss off
>
> v1->v2: updated kernel headers to uapi commit
> v2->v3: fixed alignment
> v3->v4: aligned print statements as used through the source
>
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
> ---
> ip/ipaddress.c | 40 +++++++++++++++++++++++++++++++++++-----
> 1 file changed, 35 insertions(+), 5 deletions(-)
>
Fixed the alignment issues and applied.
^ permalink raw reply
* Re: [PATCH bpf-next v6 0/5] xdp: Allow lookup into devmaps before redirect
From: Daniel Borkmann @ 2019-06-28 23:33 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, netdev
Cc: Jesper Dangaard Brouer, Alexei Starovoitov, David Miller,
Jonathan Lemon
In-Reply-To: <156171315462.9468.3367572649463706996.stgit@alrua-x1>
On 06/28/2019 11:12 AM, Toke Høiland-Jørgensen wrote:
> When using the bpf_redirect_map() helper to redirect packets from XDP, the eBPF
> program cannot currently know whether the redirect will succeed, which makes it
> impossible to gracefully handle errors. To properly fix this will probably
> require deeper changes to the way TX resources are allocated, but one thing that
> is fairly straight forward to fix is to allow lookups into devmaps, so programs
> can at least know when a redirect is *guaranteed* to fail because there is no
> entry in the map. Currently, programs work around this by keeping a shadow map
> of another type which indicates whether a map index is valid.
>
> This series contains two changes that are complementary ways to fix this issue:
>
> - Moving the map lookup into the bpf_redirect_map() helper (and caching the
> result), so the helper can return an error if no value is found in the map.
> This includes a refactoring of the devmap and cpumap code to not care about
> the index on enqueue.
>
> - Allowing regular lookups into devmaps from eBPF programs, using the read-only
> flag to make sure they don't change the values.
>
> The performance impact of the series is negligible, in the sense that I cannot
> measure it because the variance between test runs is higher than the difference
> pre/post series.
>
> Changelog:
>
> v6:
> - Factor out list handling in maps to a helper in list.h (new patch 1)
> - Rename variables in struct bpf_redirect_info (new patch 3 + patch 4)
> - Explain why we are clearing out the map in the info struct on lookup failure
> - Remove unneeded check for forwarding target in tracepoint macro
>
> v5:
> - Rebase on latest bpf-next.
> - Update documentation for bpf_redirect_map() with the new meaning of flags.
>
> v4:
> - Fix a few nits from Andrii
> - Lose the #defines in bpf.h and just compare the flags argument directly to
> XDP_TX in bpf_xdp_redirect_map().
>
> v3:
> - Adopt Jonathan's idea of using the lower two bits of the flag value as the
> return code.
> - Always do the lookup, and cache the result for use in xdp_do_redirect(); to
> achieve this, refactor the devmap and cpumap code to get rid the bitmap for
> selecting which devices to flush.
>
> v2:
> - For patch 1, make it clear that the change works for any map type.
> - For patch 2, just use the new BPF_F_RDONLY_PROG flag to make the return
> value read-only.
>
> ---
>
> Toke Høiland-Jørgensen (5):
> xskmap: Move non-standard list manipulation to helper
> devmap/cpumap: Use flush list instead of bitmap
> devmap: Rename ifindex member in bpf_redirect_info
> bpf_xdp_redirect_map: Perform map lookup in eBPF helper
> devmap: Allow map lookups from eBPF
>
>
> include/linux/filter.h | 3 +
> include/linux/list.h | 14 ++++++
> include/trace/events/xdp.h | 5 +-
> include/uapi/linux/bpf.h | 7 ++-
> kernel/bpf/cpumap.c | 105 +++++++++++++++++++----------------------
> kernel/bpf/devmap.c | 112 ++++++++++++++++++++------------------------
> kernel/bpf/verifier.c | 7 +--
> kernel/bpf/xskmap.c | 3 -
> net/core/filter.c | 60 ++++++++++++------------
> 9 files changed, 157 insertions(+), 159 deletions(-)
>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH bpf-next] selftests/bpf: fix -Wstrict-aliasing in test_sockopt_sk.c
From: Daniel Borkmann @ 2019-06-28 23:33 UTC (permalink / raw)
To: Stanislav Fomichev, netdev, bpf; +Cc: davem, ast
In-Reply-To: <20190628011233.63680-1-sdf@google.com>
On 06/28/2019 03:12 AM, Stanislav Fomichev wrote:
> Let's use union with u8[4] and u32 members for sockopt buffer,
> that should fix any possible aliasing issues.
>
> test_sockopt_sk.c: In function ‘getsetsockopt’:
> test_sockopt_sk.c:115:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
> if (*(__u32 *)buf != 0x55AA*2) {
> ^~
> test_sockopt_sk.c:116:3: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
> log_err("Unexpected getsockopt(SO_SNDBUF) 0x%x != 0x55AA*2",
> ^~~~~~~
>
> Fixes: 8a027dc0d8f5 ("selftests/bpf: add sockopt test that exercises sk helpers")
> Reported-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH v3] bpf: fix uapi bpf_prog_info fields alignment
From: Daniel Borkmann @ 2019-06-28 23:37 UTC (permalink / raw)
To: Baruch Siach, Alexei Starovoitov
Cc: Martin KaFai Lau, Song Liu, Yonghong Song, netdev, bpf,
Dmitry V . Levin, Arnd Bergmann, linux-arch, Jiri Olsa,
Geert Uytterhoeven, Linus Torvalds
In-Reply-To: <02938ce219d535a8c7c29ce796b3d6ea59c3ed15.1561694925.git.baruch@tkos.co.il>
On 06/28/2019 06:08 AM, Baruch Siach wrote:
> Merge commit 1c8c5a9d38f60 ("Merge
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next") undid the
> fix from commit 36f9814a494 ("bpf: fix uapi hole for 32 bit compat
> applications") by taking the gpl_compatible 1-bit field definition from
> commit b85fab0e67b162 ("bpf: Add gpl_compatible flag to struct
> bpf_prog_info") as is. That breaks architectures with 16-bit alignment
> like m68k. Add 31-bit pad after gpl_compatible to restore alignment of
> following fields.
>
> Thanks to Dmitry V. Levin his analysis of this bug history.
>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH 1/2] tls: remove close callback sock unlock/lock and flush_sync
From: John Fastabend @ 2019-06-29 0:20 UTC (permalink / raw)
To: Jakub Kicinski, John Fastabend; +Cc: daniel, ast, netdev, edumazet, bpf
In-Reply-To: <20190628154841.32b96fb1@cakuba.netronome.com>
Jakub Kicinski wrote:
> On Fri, 28 Jun 2019 12:40:29 -0700, John Fastabend wrote:
> > The lock() is already held when entering unhash() side so need to
> > handle this case as well,
> >
> > CPU 0 (free) CPU 1 (wq)
> >
> > lock(sk) ctx = tls_get_ctx(sk) <- need to be check null ptr
> > sk_prot->unhash()
> > set_bit()
> > cancel_work()
> > ...
> > kfree(ctx)
> > unlock(sk)
> >
> > but using cancel and doing an unlikely(!ctx) check should be
> > sufficient to handle wq.
>
> I'm not sure we can kfree ctx, the work struct itself is in it, no?
should be OK as long as we have no outstanding work. So cancel_work()
needs to be cancel_work_sync() but we can't do this in unhash so
see below...
>
> > What I'm not sure how to solve now is
> > in patch 2 of this series unhash is still calling strp_done
> > with the sock lock. Maybe we need to do a deferred release
> > like sockmap side?
>
> Right, we can't do anything that sleeps in unhash, since we're holding
> the spinlock there, not the "owner" lock.
yep.
>
> > Trying to drop the lock and then grabbing it again doesn't
> > seem right to me seems based on comment in tcp_abort we
> > could potentially "race with userspace socket closes such
> > as tcp_close". iirc I think one of the tls splats from syzbot
> > looked something like this may have happened.
> >
> > For now I'm considering adding a strp_cancel() op. Seeing
> > we are closing() the socket and tearkng down we can probably
> > be OK with throwing out strp results.
>
> But don't we have to flush the work queues before we free ctx? We'd
> need to alloc a workqueue and schedule a work to flush the other works
> and then free?
Agree, just wrote this code now and testing it. So new flow is,
lock(sk)
sk_prot->unhash()
set_bit(CLOSING)
...
tls_ctx_wq_free(ctx) <- sets up a work queue to do the *sync and kfree
unlock(sk)
FWIW sockmap has a similar problem on unhash() where it needs to
wait a RCU grace period and then do *sync operations on workqueues.
So it also schedules work to do the cleanup outside unhash() with
additional complication of waiting rcu grace period before scheduling.
The new patches are not too ugly IMO it only impacts this one
unhash() case.
>
> Why can't tls sockets exist outside of established state? If shutdown
> doesn't call close, perhaps we can add a shutdown callback? It doesn't
> seem to be called from BH?
>
Because the ulp would be shared in this case,
/* The TLS ulp is currently supported only for TCP sockets
* in ESTABLISHED state.
* Supporting sockets in LISTEN state will require us
* to modify the accept implementation to clone rather then
* share the ulp context.
*/
if (sk->sk_state != TCP_ESTABLISHED)
return -ENOTSUPP;
In general I was trying to avoid modifying core TCP layer to fix
this corner case in tls.
> Sorry for all the questions, I'm not really able to fully wrap my head
> around this. I also feel like I'm missing the sockmap piece that may
> be why you prefer unhash over disconnect.
Yep, if we try to support listening sockets we need a some more
core infrastructure to push around ulp and user_data portions of
sockets. Its not going to be nice for stable. Also at least in TLS
and sockmap case its not really needed for any use case I know
of.
>
> FWIW Davide's ULP diag support patches will require us to most likely
> free ctx with kfree_rcu(). diag only has a ref on struct sock, so if
> we want to access ctx we need RCU or to lock every socket. It's a
> little bit of an abuse of RCU, because the data under our feet may
> actually change, but the fields we dump will only get inited once
> after ulp is installed.
Ah great. I'll push a v2 this afternoon so we get syzbot running
its reproducers over it and maybe it will fit into Davide's work
as well.
^ permalink raw reply
* Re: [PATCH 1/2] tls: remove close callback sock unlock/lock and flush_sync
From: Jakub Kicinski @ 2019-06-29 0:59 UTC (permalink / raw)
To: John Fastabend; +Cc: daniel, ast, netdev, edumazet, bpf
In-Reply-To: <5d16aec74b6cd_35a32adaec47c5c457@john-XPS-13-9370.notmuch>
On Fri, 28 Jun 2019 17:20:23 -0700, John Fastabend wrote:
> > Why can't tls sockets exist outside of established state? If shutdown
> > doesn't call close, perhaps we can add a shutdown callback? It doesn't
> > seem to be called from BH?
> >
>
> Because the ulp would be shared in this case,
>
> /* The TLS ulp is currently supported only for TCP sockets
> * in ESTABLISHED state.
> * Supporting sockets in LISTEN state will require us
> * to modify the accept implementation to clone rather then
> * share the ulp context.
> */
> if (sk->sk_state != TCP_ESTABLISHED)
> return -ENOTSUPP;
>
> In general I was trying to avoid modifying core TCP layer to fix
> this corner case in tls.
I see, thanks for clarifying! I was wondering if there's anything wrong
in being in CLOSE/SYN/FIN states.
> > Sorry for all the questions, I'm not really able to fully wrap my head
> > around this. I also feel like I'm missing the sockmap piece that may
> > be why you prefer unhash over disconnect.
>
> Yep, if we try to support listening sockets we need a some more
> core infrastructure to push around ulp and user_data portions of
> sockets. Its not going to be nice for stable. Also at least in TLS
> and sockmap case its not really needed for any use case I know
> of.
IIUC we can't go from ESTABLISHED to LISTEN without calling close()
or disconnect() so I'm not clear on why are we hooking into unhash() 😕
^ permalink raw reply
* Re: [PATCH net-next 02/12] net: hns3: enable DCB when TC num is one and pfc_en is non-zero
From: Yunsheng Lin @ 2019-06-29 1:15 UTC (permalink / raw)
To: Willem de Bruijn, Huazhong Tan
Cc: David Miller, Network Development, linux-kernel, salil.mehta,
yisen.zhuang, linuxarm, Peng Li
In-Reply-To: <CA+FuTSdWa0dMz15m79SLsNAw9zkp3+3MSfKiRwKnjZ7QAyq1Uw@mail.gmail.com>
On 2019/6/29 2:47, Willem de Bruijn wrote:
> On Fri, Jun 28, 2019 at 7:53 AM Huazhong Tan <tanhuazhong@huawei.com> wrote:
>>
>> From: Yunsheng Lin <linyunsheng@huawei.com>
>>
>> Currently when TC num is one, the DCB will be disabled no matter if
>> pfc_en is non-zero or not.
>>
>> This patch enables the DCB if pfc_en is non-zero, even when TC num
>> is one.
>>
>> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
>> Signed-off-by: Peng Li <lipeng321@huawei.com>
>> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
>
>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
>> index 9edae5f..cb2fb5a 100644
>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
>> @@ -597,8 +597,10 @@ static void hclge_tm_tc_info_init(struct hclge_dev *hdev)
>> hdev->tm_info.prio_tc[i] =
>> (i >= hdev->tm_info.num_tc) ? 0 : i;
>>
>> - /* DCB is enabled if we have more than 1 TC */
>> - if (hdev->tm_info.num_tc > 1)
>> + /* DCB is enabled if we have more than 1 TC or pfc_en is
>> + * non-zero.
>> + */
>> + if (hdev->tm_info.num_tc > 1 || hdev->tm_info.pfc_en)
>
> small nit: comments that just repeat the condition are not very informative.
>
> More helpful might be to explain why the DCB should be enabled in both
> these cases. Though such detailed comments, if useful, are better left
> to the commit message usually.
Very helpful suggestion. thanks.
Will keep that in mind next time.
>
>> hdev->flag |= HCLGE_FLAG_DCB_ENABLE;
>> else
>> hdev->flag &= ~HCLGE_FLAG_DCB_ENABLE;
>> @@ -1388,6 +1390,19 @@ void hclge_tm_schd_info_update(struct hclge_dev *hdev, u8 num_tc)
>> hclge_tm_schd_info_init(hdev);
>> }
>>
>> +void hclge_tm_pfc_info_update(struct hclge_dev *hdev)
>> +{
>> + /* DCB is enabled if we have more than 1 TC or pfc_en is
>> + * non-zero.
>> + */
>> + if (hdev->tm_info.num_tc > 1 || hdev->tm_info.pfc_en)
>> + hdev->flag |= HCLGE_FLAG_DCB_ENABLE;
>> + else
>> + hdev->flag &= ~HCLGE_FLAG_DCB_ENABLE;
>> +
>> + hclge_pfc_info_init(hdev);
>> +}
>
> Avoid introducing this code duplication by defining a helper?
>
Will send out a new patch to remove the code duplication by defining a helper.
> .
>
^ permalink raw reply
* Re: net: check before dereferencing netdev_ops during busy poll
From: Josh Elsasser @ 2019-06-29 2:03 UTC (permalink / raw)
To: Sasha Levin
Cc: Matteo Croce, stable, Greg Kroah-Hartman, netdev, LKML,
David Miller
In-Reply-To: <20190628225533.GJ11506@sasha-vm>
On Jun 28, 2019, at 3:55 PM, Sasha Levin <sashal@kernel.org> wrote:
> What's the upstream commit id?
The commit wasn't needed upstream, as I only sent the original patch after
79e7fff47b7b ("net: remove support for per driver ndo_busy_poll()") had
made the fix unnecessary in Linus' tree.
May've gotten lost in the shuffle due to my poor Fixes tags. The patch in
question applied only on top of the 4.9 stable release at the time, but the
actual NPE had been around in some form since 3.11 / 0602129286705 ("net: add
low latency socket poll").
Josh
^ permalink raw reply
* Re: BUG: unable to handle kernel paging request in __do_softirq
From: syzbot @ 2019-06-29 2:10 UTC (permalink / raw)
To: ast, bp, bpf, daniel, dvyukov, hpa, jacob.jun.pan, john.fastabend,
konrad.wilk, len.brown, linux-kernel, mingo, netdev, puwen, rppt,
syzkaller-bugs, tglx, x86
In-Reply-To: <00000000000017c9e2058baf4825@google.com>
syzbot has bisected this bug to:
commit e9db4ef6bf4ca9894bb324c76e01b8f1a16b2650
Author: John Fastabend <john.fastabend@gmail.com>
Date: Sat Jun 30 13:17:47 2018 +0000
bpf: sockhash fix omitted bucket lock in sock_close
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=16aac819a00000
start commit: 29f785ff Merge branch 'fixes' of git://git.kernel.org/pub/..
git tree: upstream
final crash: https://syzkaller.appspot.com/x/report.txt?x=15aac819a00000
console output: https://syzkaller.appspot.com/x/log.txt?x=11aac819a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=e5c77f8090a3b96b
dashboard link: https://syzkaller.appspot.com/bug?extid=0b224895cb9454584de1
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1076d132a00000
Reported-by: syzbot+0b224895cb9454584de1@syzkaller.appspotmail.com
Fixes: e9db4ef6bf4c ("bpf: sockhash fix omitted bucket lock in sock_close")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* Re: [PATCH v4] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: linmiaohe @ 2019-06-29 2:13 UTC (permalink / raw)
To: David Ahern
Cc: pablo@netfilter.org, kadlec@blackhole.kfki.hu, fw@strlen.de,
davem@davemloft.net, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org, netfilter-devel@vger.kernel.org,
coreteam@netfilter.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Mingfangsen
On 6/29/19 1:05 AM, David Ahern wrote:
> On 6/28/19 3:06 AM, Miaohe Lin wrote:
> > diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c
> > b/net/ipv6/netfilter/ip6t_rpfilter.c
> > index 6bcaf7357183..3c4a1772c15f 100644
> > --- a/net/ipv6/netfilter/ip6t_rpfilter.c
> > +++ b/net/ipv6/netfilter/ip6t_rpfilter.c
> > @@ -55,6 +55,10 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
> > if (rpfilter_addr_linklocal(&iph->saddr)) {
> > lookup_flags |= RT6_LOOKUP_F_IFACE;
> > fl6.flowi6_oif = dev->ifindex;
> > + /* Set flowi6_oif for vrf devices to lookup route in l3mdev domain. */
> > + } else if (netif_is_l3_master(dev) || netif_is_l3_slave(dev)) {
> > + lookup_flags |= FLOWI_FLAG_SKIP_NH_OIF;
>
> you don't need to set that flag here. It is done by the fib_rules code as needed.
>
You're right. Fib rules code would set FLOWI_FLAG_SKIP_NH_OIF flag. But I set
it here for distinguish with the flags & XT_RPFILTER_LOOSE branch. Without
this, they do the same work and maybe should be combined. I don't want to
do that as that makes code confusing.
Is this code snipet below ok ? If so, I would delete this flag setting.
} else if (netif_is_l3_master(dev) || netif_is_l3_slave(dev)) {
fl6.flowi6_oif = dev->ifindex;
} else if ((flags & XT_RPFILTER_LOOSE) == 0)
fl6.flowi6_oif = dev->ifindex;
^ permalink raw reply
* [RFC PATCH] gve: gve_version_prefix[] can be static
From: kbuild test robot @ 2019-06-29 2:21 UTC (permalink / raw)
To: Catherine Sullivan
Cc: kbuild-all, netdev, Catherine Sullivan, Sagi Shahar, Jon Olson,
Willem de Bruijn, Luigi Rizzo
In-Reply-To: <20190626185251.205687-2-csully@google.com>
Fixes: ac22601d5bb7 ("gve: Add basic driver framework for Compute Engine Virtual NIC")
Signed-off-by: kbuild test robot <lkp@intel.com>
---
gve_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
index aa0428ef..505ca2c 100644
--- a/drivers/net/ethernet/google/gve/gve_main.c
+++ b/drivers/net/ethernet/google/gve/gve_main.c
@@ -21,7 +21,7 @@
#define GVE_VERSION_PREFIX "GVE-"
const char gve_version_str[] = GVE_VERSION;
-const char gve_version_prefix[] = GVE_VERSION_PREFIX;
+static const char gve_version_prefix[] = GVE_VERSION_PREFIX;
static int gve_alloc_counter_array(struct gve_priv *priv)
{
^ permalink raw reply related
* Re: [net-next 1/4] gve: Add basic driver framework for Compute Engine Virtual NIC
From: kbuild test robot @ 2019-06-29 2:21 UTC (permalink / raw)
To: Catherine Sullivan
Cc: kbuild-all, netdev, Catherine Sullivan, Sagi Shahar, Jon Olson,
Willem de Bruijn, Luigi Rizzo
In-Reply-To: <20190626185251.205687-2-csully@google.com>
Hi Catherine,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Catherine-Sullivan/Add-gve-driver/20190629-070444
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
drivers/net/ethernet/google/gve/gve_main.c:23:12: sparse: sparse: symbol 'gve_version_str' was not declared. Should it be static?
>> drivers/net/ethernet/google/gve/gve_main.c:24:12: sparse: sparse: symbol 'gve_version_prefix' was not declared. Should it be static?
>> drivers/net/ethernet/google/gve/gve_main.c:352:25: sparse: sparse: cast to restricted __be32
>> drivers/net/ethernet/google/gve/gve_main.c:352:25: sparse: sparse: cast to restricted __be32
>> drivers/net/ethernet/google/gve/gve_main.c:352:25: sparse: sparse: cast to restricted __be32
>> drivers/net/ethernet/google/gve/gve_main.c:352:25: sparse: sparse: cast to restricted __be32
>> drivers/net/ethernet/google/gve/gve_main.c:352:25: sparse: sparse: cast to restricted __be32
>> drivers/net/ethernet/google/gve/gve_main.c:352:25: sparse: sparse: cast to restricted __be32
drivers/net/ethernet/google/gve/gve_main.c:353:25: sparse: sparse: cast to restricted __be32
drivers/net/ethernet/google/gve/gve_main.c:353:25: sparse: sparse: cast to restricted __be32
drivers/net/ethernet/google/gve/gve_main.c:353:25: sparse: sparse: cast to restricted __be32
drivers/net/ethernet/google/gve/gve_main.c:353:25: sparse: sparse: cast to restricted __be32
drivers/net/ethernet/google/gve/gve_main.c:353:25: sparse: sparse: cast to restricted __be32
drivers/net/ethernet/google/gve/gve_main.c:353:25: sparse: sparse: cast to restricted __be32
--
>> drivers/net/ethernet/google/gve/gve_adminq.c:28:16: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected unsigned int val @@ got restricted __be3unsigned int val @@
>> drivers/net/ethernet/google/gve/gve_adminq.c:28:16: sparse: expected unsigned int val
>> drivers/net/ethernet/google/gve/gve_adminq.c:28:16: sparse: got restricted __be32 [usertype]
drivers/net/ethernet/google/gve/gve_adminq.c:68:16: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected unsigned int val @@ got restricted __be3unsigned int val @@
drivers/net/ethernet/google/gve/gve_adminq.c:68:16: sparse: expected unsigned int val
drivers/net/ethernet/google/gve/gve_adminq.c:68:16: sparse: got restricted __be32 [usertype]
>> drivers/net/ethernet/google/gve/gve_adminq.c:77:21: sparse: sparse: cast to restricted __be32
>> drivers/net/ethernet/google/gve/gve_adminq.c:77:21: sparse: sparse: cast to restricted __be32
>> drivers/net/ethernet/google/gve/gve_adminq.c:77:21: sparse: sparse: cast to restricted __be32
>> drivers/net/ethernet/google/gve/gve_adminq.c:77:21: sparse: sparse: cast to restricted __be32
>> drivers/net/ethernet/google/gve/gve_adminq.c:77:21: sparse: sparse: cast to restricted __be32
>> drivers/net/ethernet/google/gve/gve_adminq.c:77:21: sparse: sparse: cast to restricted __be32
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [net-next 4/4] gve: Add ethtool support
From: Florian Fainelli @ 2019-06-29 2:44 UTC (permalink / raw)
To: Catherine Sullivan, netdev
Cc: Sagi Shahar, Jon Olson, Willem de Bruijn, Luigi Rizzo
In-Reply-To: <20190626185251.205687-5-csully@google.com>
On 6/26/2019 11:52 AM, Catherine Sullivan wrote:
[snip]
> +static void
> +gve_get_ethtool_stats(struct net_device *netdev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + struct gve_priv *priv = netdev_priv(netdev);
> + u64 rx_pkts, rx_bytes, tx_pkts, tx_bytes;
> + int ring;
> + int i;
> +
> + ASSERT_RTNL();
> +
> + if (!netif_carrier_ok(netdev))
> + return;
> +
> + for (rx_pkts = 0, rx_bytes = 0, ring = 0;
> + ring < priv->rx_cfg.num_queues; ring++) {
> + rx_pkts += priv->rx[ring].rpackets;
> + rx_bytes += priv->rx[ring].rbytes;
> + }
> + for (tx_pkts = 0, tx_bytes = 0, ring = 0;
> + ring < priv->tx_cfg.num_queues; ring++) {
> + tx_pkts += priv->tx[ring].pkt_done;
> + tx_bytes += priv->tx[ring].bytes_done;
> + }
Maybe you do not need to support 32-bit guests with that driver, but you
might as well be correct and use the include/linux/u64_stats_sync.h
primitives to help return consistent 64-bit stats on 32-bit machines.
[snip]
> +int gve_adjust_queues(struct gve_priv *priv,
> + struct gve_queue_config new_rx_config,
> + struct gve_queue_config new_tx_config)
> +{
> + int err;
> +
> + if (netif_carrier_ok(priv->dev)) {
Should not that be netif_running()?
--
Florian
^ permalink raw reply
* Re: [PATCH 1/2] tls: remove close callback sock unlock/lock and flush_sync
From: Jakub Kicinski @ 2019-06-29 3:46 UTC (permalink / raw)
To: John Fastabend; +Cc: daniel, ast, netdev, edumazet, bpf
In-Reply-To: <20190628175925.79a763f5@cakuba.netronome.com>
On Fri, 28 Jun 2019 17:59:25 -0700, Jakub Kicinski wrote:
> > > Sorry for all the questions, I'm not really able to fully wrap my head
> > > around this. I also feel like I'm missing the sockmap piece that may
> > > be why you prefer unhash over disconnect.
> >
> > Yep, if we try to support listening sockets we need a some more
> > core infrastructure to push around ulp and user_data portions of
> > sockets. Its not going to be nice for stable. Also at least in TLS
> > and sockmap case its not really needed for any use case I know
> > of.
>
> IIUC we can't go from ESTABLISHED to LISTEN without calling close()
> or disconnect() so I'm not clear on why are we hooking into unhash() 😕
Ah, disconnect() is also called with the socket already locked.
So no BH, but still not great..
^ permalink raw reply
* [PATCH v4 bpf-next 0/9] libbpf: add bpf_link and tracing attach APIs
From: Andrii Nakryiko @ 2019-06-29 3:48 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, sdf,
songliubraving
Cc: Andrii Nakryiko
This patchset adds the following APIs to allow attaching BPF programs to
tracing entities:
- bpf_program__attach_perf_event for attaching to any opened perf event FD,
allowing users full control;
- bpf_program__attach_kprobe for attaching to kernel probes (both entry and
return probes);
- bpf_program__attach_uprobe for attaching to user probes (both entry/return);
- bpf_program__attach_tracepoint for attaching to kernel tracepoints;
- bpf_program__attach_raw_tracepoint for attaching to raw kernel tracepoint
(wrapper around bpf_raw_tracepoint_open);
This set of APIs makes libbpf more useful for tracing applications.
All attach APIs return abstract struct bpf_link that encapsulates logic of
detaching BPF program. See patch #2 for details. bpf_assoc was considered as
an alternative name for this opaque "handle", but bpf_link seems to be
appropriate semantically and is nice and short.
Pre-patch #1 makes internal libbpf_strerror_r helper function work w/ negative
error codes, lifting the burder off callers to keep track of error sign.
Patch #2 adds bpf_link abstraction.
Patch #3 adds attach_perf_event, which is the base for all other APIs.
Patch #4 adds kprobe/uprobe APIs.
Patch #5 adds tracepoint API.
Patch #6 adds raw_tracepoint API.
Patch #7 converts one existing test to use attach_perf_event.
Patch #8 adds new kprobe/uprobe tests.
Patch #9 converts some selftests currently using tracepoint to new APIs.
v3->v4:
- proper errno handling (Stanislav);
- bpf_fd -> prog_fd (Stanislav);
- switch to fprintf (Song);
v2->v3:
- added bpf_link concept (Daniel);
- didn't add generic bpf_link__attach_program for reasons described in [0];
- dropped Stanislav's Reviewed-by from patches #2-#6, in case he doesn't like
the change;
v1->v2:
- preserve errno before close() call (Stanislav);
- use libbpf_perf_event_disable_and_close in selftest (Stanislav);
- remove unnecessary memset (Stanislav);
[0] https://lore.kernel.org/bpf/CAEf4BzZ7EM5eP2eaZn7T2Yb5QgVRiwAs+epeLR1g01TTx-6m6Q@mail.gmail.com/
Andrii Nakryiko (9):
libbpf: make libbpf_strerror_r agnostic to sign of error
libbpf: introduce concept of bpf_link
libbpf: add ability to attach/detach BPF program to perf event
libbpf: add kprobe/uprobe attach API
libbpf: add tracepoint attach API
libbpf: add raw tracepoint attach API
selftests/bpf: switch test to new attach_perf_event API
selftests/bpf: add kprobe/uprobe selftests
selftests/bpf: convert existing tracepoint tests to new APIs
tools/lib/bpf/libbpf.c | 359 ++++++++++++++++++
tools/lib/bpf/libbpf.h | 21 +
tools/lib/bpf/libbpf.map | 8 +-
tools/lib/bpf/str_error.c | 2 +-
.../selftests/bpf/prog_tests/attach_probe.c | 155 ++++++++
.../bpf/prog_tests/stacktrace_build_id.c | 50 +--
.../bpf/prog_tests/stacktrace_build_id_nmi.c | 31 +-
.../selftests/bpf/prog_tests/stacktrace_map.c | 43 +--
.../bpf/prog_tests/stacktrace_map_raw_tp.c | 15 +-
.../selftests/bpf/progs/test_attach_probe.c | 55 +++
10 files changed, 644 insertions(+), 95 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
--
2.17.1
^ permalink raw reply
* [PATCH v4 bpf-next 1/9] libbpf: make libbpf_strerror_r agnostic to sign of error
From: Andrii Nakryiko @ 2019-06-29 3:48 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, sdf,
songliubraving
Cc: Andrii Nakryiko
In-Reply-To: <20190629034906.1209916-1-andriin@fb.com>
It's often inconvenient to switch sign of error when passing it into
libbpf_strerror_r. It's better for it to handle that automatically.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
tools/lib/bpf/str_error.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/str_error.c b/tools/lib/bpf/str_error.c
index 00e48ac5b806..b8064eedc177 100644
--- a/tools/lib/bpf/str_error.c
+++ b/tools/lib/bpf/str_error.c
@@ -11,7 +11,7 @@
*/
char *libbpf_strerror_r(int err, char *dst, int len)
{
- int ret = strerror_r(err, dst, len);
+ int ret = strerror_r(err < 0 ? -err : err, dst, len);
if (ret)
snprintf(dst, len, "ERROR: strerror_r(%d)=%d", err, ret);
return dst;
--
2.17.1
^ permalink raw reply related
* [PATCH v4 bpf-next 2/9] libbpf: introduce concept of bpf_link
From: Andrii Nakryiko @ 2019-06-29 3:48 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, sdf,
songliubraving
Cc: Andrii Nakryiko
In-Reply-To: <20190629034906.1209916-1-andriin@fb.com>
bpf_link is and abstraction of an association of a BPF program and one
of many possible BPF attachment points (hooks). This allows to have
uniform interface for detaching BPF programs regardless of the nature of
link and how it was created. Details of creation and setting up of
a specific bpf_link is handled by corresponding attachment methods
(bpf_program__attach_xxx) added in subsequent commits. Once successfully
created, bpf_link has to be eventually destroyed with
bpf_link__destroy(), at which point BPF program is disassociated from
a hook and all the relevant resources are freed.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
tools/lib/bpf/libbpf.c | 17 +++++++++++++++++
tools/lib/bpf/libbpf.h | 4 ++++
tools/lib/bpf/libbpf.map | 3 ++-
3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6e6ebef11ba3..455795e6f8af 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3941,6 +3941,23 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
return 0;
}
+struct bpf_link {
+ int (*destroy)(struct bpf_link *link);
+};
+
+int bpf_link__destroy(struct bpf_link *link)
+{
+ int err;
+
+ if (!link)
+ return 0;
+
+ err = link->destroy(link);
+ free(link);
+
+ return err;
+}
+
enum bpf_perf_event_ret
bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
void **copy_mem, size_t *copy_size,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index d639f47e3110..5082a5ebb0c2 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -165,6 +165,10 @@ LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path);
LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path);
LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
+struct bpf_link;
+
+LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
+
struct bpf_insn;
/*
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2c6d835620d2..3cde850fc8da 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -167,10 +167,11 @@ LIBBPF_0.0.3 {
LIBBPF_0.0.4 {
global:
+ bpf_link__destroy;
+ bpf_object__load_xattr;
btf_dump__dump_type;
btf_dump__free;
btf_dump__new;
btf__parse_elf;
- bpf_object__load_xattr;
libbpf_num_possible_cpus;
} LIBBPF_0.0.3;
--
2.17.1
^ permalink raw reply related
* [PATCH v4 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event
From: Andrii Nakryiko @ 2019-06-29 3:49 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, sdf,
songliubraving
Cc: Andrii Nakryiko
In-Reply-To: <20190629034906.1209916-1-andriin@fb.com>
bpf_program__attach_perf_event allows to attach BPF program to existing
perf event hook, providing most generic and most low-level way to attach BPF
programs. It returns struct bpf_link, which should be passed to
bpf_link__destroy to detach and free resources, associated with a link.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 61 ++++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 3 ++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 65 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 455795e6f8af..98c155ec3bfa 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -32,6 +32,7 @@
#include <linux/limits.h>
#include <linux/perf_event.h>
#include <linux/ring_buffer.h>
+#include <sys/ioctl.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/vfs.h>
@@ -3958,6 +3959,66 @@ int bpf_link__destroy(struct bpf_link *link)
return err;
}
+struct bpf_link_fd {
+ struct bpf_link link; /* has to be at the top of struct */
+ int fd; /* hook FD */
+};
+
+static int bpf_link__destroy_perf_event(struct bpf_link *link)
+{
+ struct bpf_link_fd *l = (void *)link;
+ int err;
+
+ if (l->fd < 0)
+ return 0;
+
+ err = ioctl(l->fd, PERF_EVENT_IOC_DISABLE, 0);
+ if (err)
+ err = -errno;
+
+ close(l->fd);
+ return err;
+}
+
+struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
+ int pfd)
+{
+ char errmsg[STRERR_BUFSIZE];
+ struct bpf_link_fd *link;
+ int prog_fd, err;
+
+ prog_fd = bpf_program__fd(prog);
+ if (prog_fd < 0) {
+ pr_warning("program '%s': can't attach before loaded\n",
+ bpf_program__title(prog, false));
+ return ERR_PTR(-EINVAL);
+ }
+
+ link = malloc(sizeof(*link));
+ if (!link)
+ return ERR_PTR(-ENOMEM);
+ link->link.destroy = &bpf_link__destroy_perf_event;
+ link->fd = pfd;
+
+ if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, prog_fd) < 0) {
+ err = -errno;
+ free(link);
+ pr_warning("program '%s': failed to attach to pfd %d: %s\n",
+ bpf_program__title(prog, false), pfd,
+ libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+ return ERR_PTR(err);
+ }
+ if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
+ err = -errno;
+ free(link);
+ pr_warning("program '%s': failed to enable pfd %d: %s\n",
+ bpf_program__title(prog, false), pfd,
+ libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+ return ERR_PTR(err);
+ }
+ return (struct bpf_link *)link;
+}
+
enum bpf_perf_event_ret
bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
void **copy_mem, size_t *copy_size,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 5082a5ebb0c2..1bf66c4a9330 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -169,6 +169,9 @@ struct bpf_link;
LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
+LIBBPF_API struct bpf_link *
+bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
+
struct bpf_insn;
/*
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 3cde850fc8da..756f5aa802e9 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -169,6 +169,7 @@ LIBBPF_0.0.4 {
global:
bpf_link__destroy;
bpf_object__load_xattr;
+ bpf_program__attach_perf_event;
btf_dump__dump_type;
btf_dump__free;
btf_dump__new;
--
2.17.1
^ permalink raw reply related
* [PATCH v4 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
From: Andrii Nakryiko @ 2019-06-29 3:49 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, sdf,
songliubraving
Cc: Andrii Nakryiko
In-Reply-To: <20190629034906.1209916-1-andriin@fb.com>
Add ability to attach to kernel and user probes and retprobes.
Implementation depends on perf event support for kprobes/uprobes.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 165 +++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 7 ++
tools/lib/bpf/libbpf.map | 2 +
3 files changed, 174 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 98c155ec3bfa..2f79e9563db9 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4019,6 +4019,171 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
return (struct bpf_link *)link;
}
+static int parse_value_from_file(const char *file, const char *fmt)
+{
+ char buf[STRERR_BUFSIZE];
+ int err, ret;
+ FILE *f;
+
+ f = fopen(file, "r");
+ if (!f) {
+ err = -errno;
+ pr_debug("failed to open '%s': %s\n", file,
+ libbpf_strerror_r(err, buf, sizeof(buf)));
+ fclose(f);
+ return err;
+ }
+ err = fscanf(f, fmt, &ret);
+ if (err != 1) {
+ err = err == EOF ? -EIO : -errno;
+ pr_debug("failed to parse '%s': %s\n", file,
+ libbpf_strerror_r(err, buf, sizeof(buf)));
+ fclose(f);
+ return err;
+ }
+ fclose(f);
+ return ret;
+}
+
+static int determine_kprobe_perf_type(void)
+{
+ const char *file = "/sys/bus/event_source/devices/kprobe/type";
+
+ return parse_value_from_file(file, "%d\n");
+}
+
+static int determine_uprobe_perf_type(void)
+{
+ const char *file = "/sys/bus/event_source/devices/uprobe/type";
+
+ return parse_value_from_file(file, "%d\n");
+}
+
+static int determine_kprobe_retprobe_bit(void)
+{
+ const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
+
+ return parse_value_from_file(file, "config:%d\n");
+}
+
+static int determine_uprobe_retprobe_bit(void)
+{
+ const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
+
+ return parse_value_from_file(file, "config:%d\n");
+}
+
+static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name,
+ uint64_t offset, int pid)
+{
+ struct perf_event_attr attr = {};
+ char errmsg[STRERR_BUFSIZE];
+ int type, pfd, err;
+
+ type = uprobe ? determine_uprobe_perf_type()
+ : determine_kprobe_perf_type();
+ if (type < 0) {
+ pr_warning("failed to determine %s perf type: %s\n",
+ uprobe ? "uprobe" : "kprobe",
+ libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
+ return type;
+ }
+ if (retprobe) {
+ int bit = uprobe ? determine_uprobe_retprobe_bit()
+ : determine_kprobe_retprobe_bit();
+
+ if (bit < 0) {
+ pr_warning("failed to determine %s retprobe bit: %s\n",
+ uprobe ? "uprobe" : "kprobe",
+ libbpf_strerror_r(bit, errmsg,
+ sizeof(errmsg)));
+ return bit;
+ }
+ attr.config |= 1 << bit;
+ }
+ attr.size = sizeof(attr);
+ attr.type = type;
+ attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
+ attr.config2 = offset; /* kprobe_addr or probe_offset */
+
+ /* pid filter is meaningful only for uprobes */
+ pfd = syscall(__NR_perf_event_open, &attr,
+ pid < 0 ? -1 : pid /* pid */,
+ pid == -1 ? 0 : -1 /* cpu */,
+ -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
+ if (pfd < 0) {
+ err = -errno;
+ pr_warning("%s perf_event_open() failed: %s\n",
+ uprobe ? "uprobe" : "kprobe",
+ libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+ return err;
+ }
+ return pfd;
+}
+
+struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
+ bool retprobe,
+ const char *func_name)
+{
+ char errmsg[STRERR_BUFSIZE];
+ struct bpf_link *link;
+ int pfd, err;
+
+ pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
+ 0 /* offset */, -1 /* pid */);
+ if (pfd < 0) {
+ pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
+ bpf_program__title(prog, false),
+ retprobe ? "kretprobe" : "kprobe", func_name,
+ libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
+ return ERR_PTR(pfd);
+ }
+ link = bpf_program__attach_perf_event(prog, pfd);
+ if (IS_ERR(link)) {
+ close(pfd);
+ err = PTR_ERR(link);
+ pr_warning("program '%s': failed to attach to %s '%s': %s\n",
+ bpf_program__title(prog, false),
+ retprobe ? "kretprobe" : "kprobe", func_name,
+ libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+ return link;
+ }
+ return link;
+}
+
+struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
+ bool retprobe, pid_t pid,
+ const char *binary_path,
+ size_t func_offset)
+{
+ char errmsg[STRERR_BUFSIZE];
+ struct bpf_link *link;
+ int pfd, err;
+
+ pfd = perf_event_open_probe(true /* uprobe */, retprobe,
+ binary_path, func_offset, pid);
+ if (pfd < 0) {
+ pr_warning("program '%s': failed to create %s '%s:0x%zx' perf event: %s\n",
+ bpf_program__title(prog, false),
+ retprobe ? "uretprobe" : "uprobe",
+ binary_path, func_offset,
+ libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
+ return ERR_PTR(pfd);
+ }
+ link = bpf_program__attach_perf_event(prog, pfd);
+ if (IS_ERR(link)) {
+ close(pfd);
+ err = PTR_ERR(link);
+ pr_warning("program '%s': failed to attach to %s '%s:0x%zx': %s\n",
+ bpf_program__title(prog, false),
+ retprobe ? "uretprobe" : "uprobe",
+ binary_path, func_offset,
+ libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+ return link;
+ }
+ return link;
+}
+
enum bpf_perf_event_ret
bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
void **copy_mem, size_t *copy_size,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 1bf66c4a9330..bd767cc11967 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -171,6 +171,13 @@ LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
LIBBPF_API struct bpf_link *
bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
+LIBBPF_API struct bpf_link *
+bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
+ const char *func_name);
+LIBBPF_API struct bpf_link *
+bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
+ pid_t pid, const char *binary_path,
+ size_t func_offset);
struct bpf_insn;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 756f5aa802e9..57a40fb60718 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -169,7 +169,9 @@ LIBBPF_0.0.4 {
global:
bpf_link__destroy;
bpf_object__load_xattr;
+ bpf_program__attach_kprobe;
bpf_program__attach_perf_event;
+ bpf_program__attach_uprobe;
btf_dump__dump_type;
btf_dump__free;
btf_dump__new;
--
2.17.1
^ permalink raw reply related
* [PATCH v4 bpf-next 5/9] libbpf: add tracepoint attach API
From: Andrii Nakryiko @ 2019-06-29 3:49 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, sdf,
songliubraving
Cc: Andrii Nakryiko
In-Reply-To: <20190629034906.1209916-1-andriin@fb.com>
Allow attaching BPF programs to kernel tracepoint BPF hooks specified by
category and name.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
tools/lib/bpf/libbpf.c | 79 ++++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 4 ++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 84 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2f79e9563db9..8ad4f915df38 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4184,6 +4184,85 @@ struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
return link;
}
+static int determine_tracepoint_id(const char *tp_category,
+ const char *tp_name)
+{
+ char file[PATH_MAX];
+ int ret;
+
+ ret = snprintf(file, sizeof(file),
+ "/sys/kernel/debug/tracing/events/%s/%s/id",
+ tp_category, tp_name);
+ if (ret < 0)
+ return -errno;
+ if (ret >= sizeof(file)) {
+ pr_debug("tracepoint %s/%s path is too long\n",
+ tp_category, tp_name);
+ return -E2BIG;
+ }
+ return parse_value_from_file(file, "%d\n");
+}
+
+static int perf_event_open_tracepoint(const char *tp_category,
+ const char *tp_name)
+{
+ struct perf_event_attr attr = {};
+ char errmsg[STRERR_BUFSIZE];
+ int tp_id, pfd, err;
+
+ tp_id = determine_tracepoint_id(tp_category, tp_name);
+ if (tp_id < 0) {
+ pr_warning("failed to determine tracepoint '%s/%s' perf ID: %s\n",
+ tp_category, tp_name,
+ libbpf_strerror_r(tp_id, errmsg, sizeof(errmsg)));
+ return tp_id;
+ }
+
+ attr.type = PERF_TYPE_TRACEPOINT;
+ attr.size = sizeof(attr);
+ attr.config = tp_id;
+
+ pfd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, 0 /* cpu */,
+ -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
+ if (pfd < 0) {
+ err = -errno;
+ pr_warning("tracepoint '%s/%s' perf_event_open() failed: %s\n",
+ tp_category, tp_name,
+ libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+ return err;
+ }
+ return pfd;
+}
+
+struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
+ const char *tp_category,
+ const char *tp_name)
+{
+ char errmsg[STRERR_BUFSIZE];
+ struct bpf_link *link;
+ int pfd, err;
+
+ pfd = perf_event_open_tracepoint(tp_category, tp_name);
+ if (pfd < 0) {
+ pr_warning("program '%s': failed to create tracepoint '%s/%s' perf event: %s\n",
+ bpf_program__title(prog, false),
+ tp_category, tp_name,
+ libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
+ return ERR_PTR(pfd);
+ }
+ link = bpf_program__attach_perf_event(prog, pfd);
+ if (IS_ERR(link)) {
+ close(pfd);
+ err = PTR_ERR(link);
+ pr_warning("program '%s': failed to attach to tracepoint '%s/%s': %s\n",
+ bpf_program__title(prog, false),
+ tp_category, tp_name,
+ libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+ return link;
+ }
+ return link;
+}
+
enum bpf_perf_event_ret
bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
void **copy_mem, size_t *copy_size,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index bd767cc11967..60611f4b4e1d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -178,6 +178,10 @@ LIBBPF_API struct bpf_link *
bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
pid_t pid, const char *binary_path,
size_t func_offset);
+LIBBPF_API struct bpf_link *
+bpf_program__attach_tracepoint(struct bpf_program *prog,
+ const char *tp_category,
+ const char *tp_name);
struct bpf_insn;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 57a40fb60718..3c618b75ef65 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
bpf_object__load_xattr;
bpf_program__attach_kprobe;
bpf_program__attach_perf_event;
+ bpf_program__attach_tracepoint;
bpf_program__attach_uprobe;
btf_dump__dump_type;
btf_dump__free;
--
2.17.1
^ permalink raw reply related
* [PATCH v4 bpf-next 6/9] libbpf: add raw tracepoint attach API
From: Andrii Nakryiko @ 2019-06-29 3:49 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, sdf,
songliubraving
Cc: Andrii Nakryiko
In-Reply-To: <20190629034906.1209916-1-andriin@fb.com>
Add a wrapper utilizing bpf_link "infrastructure" to allow attaching BPF
programs to raw tracepoints.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
tools/lib/bpf/libbpf.c | 37 +++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 3 +++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 41 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 8ad4f915df38..f8c7a7ecb35e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4263,6 +4263,43 @@ struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
return link;
}
+static int bpf_link__destroy_fd(struct bpf_link *link)
+{
+ struct bpf_link_fd *l = (void *)link;
+
+ return close(l->fd);
+}
+
+struct bpf_link *bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
+ const char *tp_name)
+{
+ char errmsg[STRERR_BUFSIZE];
+ struct bpf_link_fd *link;
+ int prog_fd, pfd;
+
+ prog_fd = bpf_program__fd(prog);
+ if (prog_fd < 0) {
+ pr_warning("program '%s': can't attach before loaded\n",
+ bpf_program__title(prog, false));
+ return ERR_PTR(-EINVAL);
+ }
+
+ link = malloc(sizeof(*link));
+ link->link.destroy = &bpf_link__destroy_fd;
+
+ pfd = bpf_raw_tracepoint_open(tp_name, prog_fd);
+ if (pfd < 0) {
+ pfd = -errno;
+ free(link);
+ pr_warning("program '%s': failed to attach to raw tracepoint '%s': %s\n",
+ bpf_program__title(prog, false), tp_name,
+ libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
+ return ERR_PTR(pfd);
+ }
+ link->fd = pfd;
+ return (struct bpf_link *)link;
+}
+
enum bpf_perf_event_ret
bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
void **copy_mem, size_t *copy_size,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 60611f4b4e1d..f55933784f95 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -182,6 +182,9 @@ LIBBPF_API struct bpf_link *
bpf_program__attach_tracepoint(struct bpf_program *prog,
const char *tp_category,
const char *tp_name);
+LIBBPF_API struct bpf_link *
+bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
+ const char *tp_name);
struct bpf_insn;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 3c618b75ef65..e6b7d4edbc93 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
bpf_object__load_xattr;
bpf_program__attach_kprobe;
bpf_program__attach_perf_event;
+ bpf_program__attach_raw_tracepoint;
bpf_program__attach_tracepoint;
bpf_program__attach_uprobe;
btf_dump__dump_type;
--
2.17.1
^ permalink raw reply related
* [PATCH v4 bpf-next 8/9] selftests/bpf: add kprobe/uprobe selftests
From: Andrii Nakryiko @ 2019-06-29 3:49 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, sdf,
songliubraving
Cc: Andrii Nakryiko
In-Reply-To: <20190629034906.1209916-1-andriin@fb.com>
Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
expected.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
.../selftests/bpf/prog_tests/attach_probe.c | 155 ++++++++++++++++++
.../selftests/bpf/progs/test_attach_probe.c | 55 +++++++
2 files changed, 210 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
new file mode 100644
index 000000000000..f22929063c58
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+
+ssize_t get_base_addr() {
+ size_t start;
+ char buf[256];
+ FILE *f;
+
+ f = fopen("/proc/self/maps", "r");
+ if (!f)
+ return -errno;
+
+ while (fscanf(f, "%zx-%*x %s %*s\n", &start, buf) == 2) {
+ if (strcmp(buf, "r-xp") == 0) {
+ fclose(f);
+ return start;
+ }
+ }
+
+ fclose(f);
+ return -EINVAL;
+}
+
+void test_attach_probe(void)
+{
+ const char *kprobe_name = "kprobe/sys_nanosleep";
+ const char *kretprobe_name = "kretprobe/sys_nanosleep";
+ const char *uprobe_name = "uprobe/trigger_func";
+ const char *uretprobe_name = "uretprobe/trigger_func";
+ const int kprobe_idx = 0, kretprobe_idx = 1;
+ const int uprobe_idx = 2, uretprobe_idx = 3;
+ const char *file = "./test_attach_probe.o";
+ struct bpf_program *kprobe_prog, *kretprobe_prog;
+ struct bpf_program *uprobe_prog, *uretprobe_prog;
+ struct bpf_object *obj;
+ int err, prog_fd, duration = 0, res;
+ struct bpf_link *kprobe_link = NULL;
+ struct bpf_link *kretprobe_link = NULL;
+ struct bpf_link *uprobe_link = NULL;
+ struct bpf_link *uretprobe_link = NULL;
+ int results_map_fd;
+ size_t uprobe_offset;
+ ssize_t base_addr;
+
+ base_addr = get_base_addr();
+ if (CHECK(base_addr < 0, "get_base_addr",
+ "failed to find base addr: %zd", base_addr))
+ return;
+ uprobe_offset = (size_t)&get_base_addr - base_addr;
+
+ /* load programs */
+ err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
+ if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
+ return;
+
+ kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
+ if (CHECK(!kprobe_prog, "find_probe",
+ "prog '%s' not found\n", kprobe_name))
+ goto cleanup;
+ kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
+ if (CHECK(!kretprobe_prog, "find_probe",
+ "prog '%s' not found\n", kretprobe_name))
+ goto cleanup;
+ uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
+ if (CHECK(!uprobe_prog, "find_probe",
+ "prog '%s' not found\n", uprobe_name))
+ goto cleanup;
+ uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
+ if (CHECK(!uretprobe_prog, "find_probe",
+ "prog '%s' not found\n", uretprobe_name))
+ goto cleanup;
+
+ /* load maps */
+ results_map_fd = bpf_find_map(__func__, obj, "results_map");
+ if (CHECK(results_map_fd < 0, "find_results_map",
+ "err %d\n", results_map_fd))
+ goto cleanup;
+
+ kprobe_link = bpf_program__attach_kprobe(kprobe_prog,
+ false /* retprobe */,
+ "sys_nanosleep");
+ if (CHECK(IS_ERR(kprobe_link), "attach_kprobe",
+ "err %ld\n", PTR_ERR(kprobe_link)))
+ goto cleanup;
+
+ kretprobe_link = bpf_program__attach_kprobe(kretprobe_prog,
+ true /* retprobe */,
+ "sys_nanosleep");
+ if (CHECK(IS_ERR(kretprobe_link), "attach_kretprobe",
+ "err %ld\n", PTR_ERR(kretprobe_link)))
+ goto cleanup;
+
+ uprobe_link = bpf_program__attach_uprobe(uprobe_prog,
+ false /* retprobe */,
+ 0 /* self pid */,
+ "/proc/self/exe",
+ uprobe_offset);
+ if (CHECK(IS_ERR(uprobe_link), "attach_uprobe",
+ "err %ld\n", PTR_ERR(uprobe_link)))
+ goto cleanup;
+
+ uretprobe_link = bpf_program__attach_uprobe(uretprobe_prog,
+ true /* retprobe */,
+ -1 /* any pid */,
+ "/proc/self/exe",
+ uprobe_offset);
+ if (CHECK(IS_ERR(uretprobe_link), "attach_uretprobe",
+ "err %ld\n", PTR_ERR(uretprobe_link)))
+ goto cleanup;
+
+ /* trigger & validate kprobe && kretprobe */
+ usleep(1);
+
+ err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
+ if (CHECK(err, "get_kprobe_res",
+ "failed to get kprobe res: %d\n", err))
+ goto cleanup;
+ if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
+ "wrong kprobe res: %d\n", res))
+ goto cleanup;
+
+ err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
+ if (CHECK(err, "get_kretprobe_res",
+ "failed to get kretprobe res: %d\n", err))
+ goto cleanup;
+ if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
+ "wrong kretprobe res: %d\n", res))
+ goto cleanup;
+
+ /* trigger & validate uprobe & uretprobe */
+ get_base_addr();
+
+ err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
+ if (CHECK(err, "get_uprobe_res",
+ "failed to get uprobe res: %d\n", err))
+ goto cleanup;
+ if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
+ "wrong uprobe res: %d\n", res))
+ goto cleanup;
+
+ err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
+ if (CHECK(err, "get_uretprobe_res",
+ "failed to get uretprobe res: %d\n", err))
+ goto cleanup;
+ if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
+ "wrong uretprobe res: %d\n", res))
+ goto cleanup;
+
+cleanup:
+ bpf_link__destroy(kprobe_link);
+ bpf_link__destroy(kretprobe_link);
+ bpf_link__destroy(uprobe_link);
+ bpf_link__destroy(uretprobe_link);
+ bpf_object__close(obj);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
new file mode 100644
index 000000000000..7a7c5cd728c8
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2017 Facebook
+
+#include <linux/ptrace.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct {
+ int type;
+ int max_entries;
+ int *key;
+ int *value;
+} results_map SEC(".maps") = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .max_entries = 4,
+};
+
+SEC("kprobe/sys_nanosleep")
+int handle_sys_nanosleep_entry(struct pt_regs *ctx)
+{
+ const int key = 0, value = 1;
+
+ bpf_map_update_elem(&results_map, &key, &value, 0);
+ return 0;
+}
+
+SEC("kretprobe/sys_nanosleep")
+int handle_sys_getpid_return(struct pt_regs *ctx)
+{
+ const int key = 1, value = 2;
+
+ bpf_map_update_elem(&results_map, &key, &value, 0);
+ return 0;
+}
+
+SEC("uprobe/trigger_func")
+int handle_uprobe_entry(struct pt_regs *ctx)
+{
+ const int key = 2, value = 3;
+
+ bpf_map_update_elem(&results_map, &key, &value, 0);
+ return 0;
+}
+
+SEC("uretprobe/trigger_func")
+int handle_uprobe_return(struct pt_regs *ctx)
+{
+ const int key = 3, value = 4;
+
+ bpf_map_update_elem(&results_map, &key, &value, 0);
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox