From: Daniel Borkmann <daniel@iogearbox•net>
To: Saeed Mahameed <saeedm@dev•mellanox.co.il>
Cc: "David S. Miller" <davem@davemloft•net>,
Alexei Starovoitov <alexei.starovoitov@gmail•com>,
Brenden Blanco <bblanco@plumgrid•com>,
zhiyisun@gmail•com, Rana Shahout <ranas@mellanox•com>,
Saeed Mahameed <saeedm@mellanox•com>,
Linux Netdev List <netdev@vger•kernel.org>
Subject: Re: [PATCH net-next v2 3/4] bpf, mlx5: drop priv->xdp_prog reference on netdev cleanup
Date: Wed, 16 Nov 2016 16:45:21 +0100 [thread overview]
Message-ID: <582C7F11.7000006@iogearbox.net> (raw)
In-Reply-To: <CALzJLG9Rzds3H-wXnjCg=_2oGV-33TY714Hx0n+NyVsZzcBBfA@mail.gmail.com>
On 11/16/2016 01:51 PM, Saeed Mahameed wrote:
> On Wed, Nov 16, 2016 at 2:04 AM, Daniel Borkmann <daniel@iogearbox•net> wrote:
>> mlx5e_xdp_set() is currently the only place where we drop reference on the
>> prog sitting in priv->xdp_prog when it's exchanged by a new one. We also
>> need to make sure that we eventually release that reference, for example,
>> in case the netdev is dismantled.
>>
>> Fixes: 86994156c736 ("net/mlx5e: XDP fast RX drop bpf programs support")
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox•net>
>> ---
>> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> index cf26672..60fe54c 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> @@ -3715,6 +3715,9 @@ static void mlx5e_nic_cleanup(struct mlx5e_priv *priv)
>>
>> if (MLX5_CAP_GEN(mdev, vport_group_manager))
>> mlx5_eswitch_unregister_vport_rep(esw, 0);
>> +
>> + if (priv->xdp_prog)
>> + bpf_prog_put(priv->xdp_prog);
>> }
>
> I thought that on unregister_netdev ndo_xdp_set will be called with
> NULL prog to cleanup. like any other resources (Vlans/mac_lists/
> etc..), why xdp should be different ?
> Anyway if this is the case, I am ok with this fix, you can even send
> it to net (it looks like a serious leak).
The only interaction with ndo_xdp() right now is dev_change_xdp_fd()
and the currently a bit terse dump via rtnl_xdp_fill(). The latter
only tells whether something is actually attached and will have more
info in near future, but doesn't alter anything.
dev_change_xdp_fd() is only triggered from user side via netlink when
IFLA_XDP container attr is around, so no automatic cleanup here. This
means as per documentation in enum xdp_netdev_command, that the driver
has full ownership, thus needs to bpf_prog_put().
next prev parent reply other threads:[~2016-11-16 15:45 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-16 0:04 [PATCH net-next v2 0/4] Couple of BPF refcount fixes for mlx5 Daniel Borkmann
2016-11-16 0:04 ` [PATCH net-next v2 1/4] bpf, mlx5: fix mlx5e_create_rq taking reference on prog Daniel Borkmann
2016-11-16 0:04 ` [PATCH net-next v2 2/4] bpf, mlx5: fix various refcount issues in mlx5e_xdp_set Daniel Borkmann
2016-11-16 12:25 ` Saeed Mahameed
2016-11-16 13:54 ` Daniel Borkmann
2016-11-16 14:30 ` Saeed Mahameed
2016-11-16 15:26 ` Daniel Borkmann
2016-11-17 9:48 ` Saeed Mahameed
2016-11-17 20:03 ` Daniel Borkmann
2016-11-16 0:04 ` [PATCH net-next v2 3/4] bpf, mlx5: drop priv->xdp_prog reference on netdev cleanup Daniel Borkmann
2016-11-16 12:51 ` Saeed Mahameed
2016-11-16 15:45 ` Daniel Borkmann [this message]
2016-11-16 15:55 ` Daniel Borkmann
2016-11-17 9:34 ` Saeed Mahameed
2016-11-16 0:04 ` [PATCH net-next v2 4/4] bpf: add __must_check attributes to refcount manipulating helpers Daniel Borkmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=582C7F11.7000006@iogearbox.net \
--to=daniel@iogearbox$(echo .)net \
--cc=alexei.starovoitov@gmail$(echo .)com \
--cc=bblanco@plumgrid$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=ranas@mellanox$(echo .)com \
--cc=saeedm@dev$(echo .)mellanox.co.il \
--cc=saeedm@mellanox$(echo .)com \
--cc=zhiyisun@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox