diff mbox series

[net-next,v2,3/6] tsnep: Support XDP BPF program setup

Message ID 20221208054045.3600-4-gerhard@engleder-embedded.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series tsnep: XDP support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 11 of 11 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Gerhard Engleder Dec. 8, 2022, 5:40 a.m. UTC 
Implement setup of BPF programs for XDP RX path with command
XDP_SETUP_PROG of ndo_bpf(). This is prework for XDP RX path support.

Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
---
 drivers/net/ethernet/engleder/Makefile     |  2 +-
 drivers/net/ethernet/engleder/tsnep.h      | 13 +++++++++++
 drivers/net/ethernet/engleder/tsnep_main.c | 17 ++++++++++++--
 drivers/net/ethernet/engleder/tsnep_xdp.c  | 27 ++++++++++++++++++++++
 4 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/ethernet/engleder/tsnep_xdp.c

Comments

Saeed Mahameed Dec. 9, 2022, 12:43 a.m. UTC | #1 
On 08 Dec 06:40, Gerhard Engleder wrote:
>Implement setup of BPF programs for XDP RX path with command
>XDP_SETUP_PROG of ndo_bpf(). This is prework for XDP RX path support.
>
>Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>

[ ... ]

>+int tsnep_xdp_setup_prog(struct tsnep_adapter *adapter, struct bpf_prog *prog,
>+			 struct netlink_ext_ack *extack)
>+{
>+	struct net_device *dev = adapter->netdev;
>+	bool if_running = netif_running(dev);
>+	struct bpf_prog *old_prog;
>+
>+	if (if_running)
>+		tsnep_netdev_close(dev);
>+
>+	old_prog = xchg(&adapter->xdp_prog, prog);
>+	if (old_prog)
>+		bpf_prog_put(old_prog);
>+
>+	if (if_running)
>+		tsnep_netdev_open(dev);

this could fail silently, and then cause double free, when close ndo 
will be called, the stack won't be aware of the closed state..
Gerhard Engleder Dec. 9, 2022, 8:06 a.m. UTC | #2 
On 09.12.22 01:43, Saeed Mahameed wrote:
>> +int tsnep_xdp_setup_prog(struct tsnep_adapter *adapter, struct 
>> bpf_prog *prog,
>> +             struct netlink_ext_ack *extack)
>> +{
>> +    struct net_device *dev = adapter->netdev;
>> +    bool if_running = netif_running(dev);
>> +    struct bpf_prog *old_prog;
>> +
>> +    if (if_running)
>> +        tsnep_netdev_close(dev);
>> +
>> +    old_prog = xchg(&adapter->xdp_prog, prog);
>> +    if (old_prog)
>> +        bpf_prog_put(old_prog);
>> +
>> +    if (if_running)
>> +        tsnep_netdev_open(dev);
> 
> this could fail silently, and then cause double free, when close ndo 
> will be called, the stack won't be aware of the closed state..

I will ensure that no double free will happen when ndo_close is called.

Gerhard
Gerhard Engleder Dec. 21, 2022, 8:19 p.m. UTC | #3 
On 09.12.22 09:06, Gerhard Engleder wrote:
> On 09.12.22 01:43, Saeed Mahameed wrote:
>>> +int tsnep_xdp_setup_prog(struct tsnep_adapter *adapter, struct 
>>> bpf_prog *prog,
>>> +             struct netlink_ext_ack *extack)
>>> +{
>>> +    struct net_device *dev = adapter->netdev;
>>> +    bool if_running = netif_running(dev);
>>> +    struct bpf_prog *old_prog;
>>> +
>>> +    if (if_running)
>>> +        tsnep_netdev_close(dev);
>>> +
>>> +    old_prog = xchg(&adapter->xdp_prog, prog);
>>> +    if (old_prog)
>>> +        bpf_prog_put(old_prog);
>>> +
>>> +    if (if_running)
>>> +        tsnep_netdev_open(dev);
>>
>> this could fail silently, and then cause double free, when close ndo 
>> will be called, the stack won't be aware of the closed state..
> 
> I will ensure that no double free will happen when ndo_close is called.

Other drivers like igc/igb/netsec/stmmac also fail silently and
I don't see any measures against double free in this drivers.
mvneta forwards the return value of open. How are these drivers
solving this issue? I cannot find this detail, but I also do not
believe that all this drivers are buggy.

gerhard
diff mbox series

Patch

diff --git a/drivers/net/ethernet/engleder/Makefile b/drivers/net/ethernet/engleder/Makefile
index b6e3b16623de..0901801cfcc9 100644
--- a/drivers/net/ethernet/engleder/Makefile
+++ b/drivers/net/ethernet/engleder/Makefile
@@ -6,5 +6,5 @@ 
 obj-$(CONFIG_TSNEP) += tsnep.o
 
 tsnep-objs := tsnep_main.o tsnep_ethtool.o tsnep_ptp.o tsnep_tc.o \
-	      tsnep_rxnfc.o $(tsnep-y)
+	      tsnep_rxnfc.o tsnep_xdp.o $(tsnep-y)
 tsnep-$(CONFIG_TSNEP_SELFTESTS) += tsnep_selftests.o
diff --git a/drivers/net/ethernet/engleder/tsnep.h b/drivers/net/ethernet/engleder/tsnep.h
index 29b04127f529..0e7fc36a64e1 100644
--- a/drivers/net/ethernet/engleder/tsnep.h
+++ b/drivers/net/ethernet/engleder/tsnep.h
@@ -183,6 +183,8 @@  struct tsnep_adapter {
 	int rxnfc_count;
 	int rxnfc_max;
 
+	struct bpf_prog *xdp_prog;
+
 	int num_tx_queues;
 	struct tsnep_tx tx[TSNEP_MAX_QUEUES];
 	int num_rx_queues;
@@ -192,6 +194,9 @@  struct tsnep_adapter {
 	struct tsnep_queue queue[TSNEP_MAX_QUEUES];
 };
 
+int tsnep_netdev_open(struct net_device *netdev);
+int tsnep_netdev_close(struct net_device *netdev);
+
 extern const struct ethtool_ops tsnep_ethtool_ops;
 
 int tsnep_ptp_init(struct tsnep_adapter *adapter);
@@ -215,6 +220,14 @@  int tsnep_rxnfc_add_rule(struct tsnep_adapter *adapter,
 int tsnep_rxnfc_del_rule(struct tsnep_adapter *adapter,
 			 struct ethtool_rxnfc *cmd);
 
+int tsnep_xdp_setup_prog(struct tsnep_adapter *adapter, struct bpf_prog *prog,
+			 struct netlink_ext_ack *extack);
+
+static inline bool tsnep_xdp_is_enabled(struct tsnep_adapter *adapter)
+{
+	return !!adapter->xdp_prog;
+}
+
 #if IS_ENABLED(CONFIG_TSNEP_SELFTESTS)
 int tsnep_ethtool_get_test_count(void);
 void tsnep_ethtool_get_test_strings(u8 *data);
diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
index b97cfd5fa1fa..0a1957259df8 100644
--- a/drivers/net/ethernet/engleder/tsnep_main.c
+++ b/drivers/net/ethernet/engleder/tsnep_main.c
@@ -1233,7 +1233,7 @@  static void tsnep_free_irq(struct tsnep_queue *queue, bool first)
 	memset(queue->name, 0, sizeof(queue->name));
 }
 
-static int tsnep_netdev_open(struct net_device *netdev)
+int tsnep_netdev_open(struct net_device *netdev)
 {
 	struct tsnep_adapter *adapter = netdev_priv(netdev);
 	int i;
@@ -1312,7 +1312,7 @@  static int tsnep_netdev_open(struct net_device *netdev)
 	return retval;
 }
 
-static int tsnep_netdev_close(struct net_device *netdev)
+int tsnep_netdev_close(struct net_device *netdev)
 {
 	struct tsnep_adapter *adapter = netdev_priv(netdev);
 	int i;
@@ -1481,6 +1481,18 @@  static ktime_t tsnep_netdev_get_tstamp(struct net_device *netdev,
 	return ns_to_ktime(timestamp);
 }
 
+static int tsnep_netdev_bpf(struct net_device *dev, struct netdev_bpf *bpf)
+{
+	struct tsnep_adapter *adapter = netdev_priv(dev);
+
+	switch (bpf->command) {
+	case XDP_SETUP_PROG:
+		return tsnep_xdp_setup_prog(adapter, bpf->prog, bpf->extack);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static int tsnep_netdev_xdp_xmit(struct net_device *dev, int n,
 				 struct xdp_frame **xdp, u32 flags)
 {
@@ -1533,6 +1545,7 @@  static const struct net_device_ops tsnep_netdev_ops = {
 	.ndo_set_features = tsnep_netdev_set_features,
 	.ndo_get_tstamp = tsnep_netdev_get_tstamp,
 	.ndo_setup_tc = tsnep_tc_setup,
+	.ndo_bpf = tsnep_netdev_bpf,
 	.ndo_xdp_xmit = tsnep_netdev_xdp_xmit,
 };
 
diff --git a/drivers/net/ethernet/engleder/tsnep_xdp.c b/drivers/net/ethernet/engleder/tsnep_xdp.c
new file mode 100644
index 000000000000..02d84dfbdde4
--- /dev/null
+++ b/drivers/net/ethernet/engleder/tsnep_xdp.c
@@ -0,0 +1,27 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2022 Gerhard Engleder <gerhard@engleder-embedded.com> */
+
+#include <linux/if_vlan.h>
+#include <net/xdp_sock_drv.h>
+
+#include "tsnep.h"
+
+int tsnep_xdp_setup_prog(struct tsnep_adapter *adapter, struct bpf_prog *prog,
+			 struct netlink_ext_ack *extack)
+{
+	struct net_device *dev = adapter->netdev;
+	bool if_running = netif_running(dev);
+	struct bpf_prog *old_prog;
+
+	if (if_running)
+		tsnep_netdev_close(dev);
+
+	old_prog = xchg(&adapter->xdp_prog, prog);
+	if (old_prog)
+		bpf_prog_put(old_prog);
+
+	if (if_running)
+		tsnep_netdev_open(dev);
+
+	return 0;
+}