| Message ID | 20230109191523.12070-11-gerhard@engleder-embedded.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | tsnep: XDP support | expand |
On Mon, 2023-01-09 at 20:15 +0100, Gerhard Engleder wrote: > Implement setup of BPF programs for XDP RX path with command > XDP_SETUP_PROG of ndo_bpf(). This is the final step for XDP RX path > support. > > tsnep_netdev_close() is called directly during BPF program setup. Add > netif_carrier_off() and netif_tx_stop_all_queues() calls to signal to > network stack that device is down. Otherwise network stack would > continue transmitting pakets. > > Return value of tsnep_netdev_open() is not checked during BPF program > setup like in other drivers. Forwarding the return value would result in > a bpf_prog_put() call in dev_xdp_install(), which would make removal of > BPF program necessary. > > If tsnep_netdev_open() fails during BPF program setup, then the network > stack would call tsnep_netdev_close() anyway. Thus, tsnep_netdev_close() > checks now if device is already down. > > Additionally remove $(tsnep-y) from $(tsnep-objs) because it is added > automatically. > > Test results with A53 1.2GHz: > > XDP_DROP (samples/bpf/xdp1) > proto 17: 883878 pkt/s > > XDP_TX (samples/bpf/xdp2) > proto 17: 255693 pkt/s > > XDP_REDIRECT (samples/bpf/xdpsock) > sock0@eth2:0 rxdrop xdp-drv > pps pkts 1.00 > rx 855,582 5,404,523 > tx 0 0 > > XDP_REDIRECT (samples/bpf/xdp_redirect) > eth2->eth1 613,267 rx/s 0 err,drop/s 613,272 xmit/s > > Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com> > --- > drivers/net/ethernet/engleder/Makefile | 2 +- > drivers/net/ethernet/engleder/tsnep.h | 6 +++++ > drivers/net/ethernet/engleder/tsnep_main.c | 25 ++++++++++++++++--- > drivers/net/ethernet/engleder/tsnep_xdp.c | 29 ++++++++++++++++++++++ > 4 files changed, 58 insertions(+), 4 deletions(-) > create mode 100644 drivers/net/ethernet/engleder/tsnep_xdp.c > > <...> > --- a/drivers/net/ethernet/engleder/tsnep_main.c > +++ b/drivers/net/ethernet/engleder/tsnep_main.c > @@ -1373,7 +1373,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 tx_queue_index = 0; > @@ -1436,6 +1436,8 @@ static int tsnep_netdev_open(struct net_device *netdev) > tsnep_enable_irq(adapter, adapter->queue[i].irq_mask); > } > > + netif_tx_start_all_queues(adapter->netdev); > + > clear_bit(__TSNEP_DOWN, &adapter->state); > > return 0; > @@ -1457,12 +1459,16 @@ 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; > > - set_bit(__TSNEP_DOWN, &adapter->state); > + if (test_and_set_bit(__TSNEP_DOWN, &adapter->state)) > + return 0; > + > + netif_carrier_off(netdev); > + netif_tx_stop_all_queues(netdev); > As I called out earlier the __TSNEP_DOWN is just !IFF_UP so you don't need that bit. The fact that netif_carrier_off is here also points out the fact that the code in the Tx path isn't needed regarding __TSNEP_DOWN and you can probably just check netif_carrier_ok if you need the check. > tsnep_disable_irq(adapter, ECM_INT_LINK); > tsnep_phy_close(adapter); > @@ -1627,6 +1633,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) > { > @@ -1677,6 +1695,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, > }; > >
On 10.01.23 18:33, Alexander H Duyck wrote: > On Mon, 2023-01-09 at 20:15 +0100, Gerhard Engleder wrote: >> Implement setup of BPF programs for XDP RX path with command >> XDP_SETUP_PROG of ndo_bpf(). This is the final step for XDP RX path >> support. >> >> tsnep_netdev_close() is called directly during BPF program setup. Add >> netif_carrier_off() and netif_tx_stop_all_queues() calls to signal to >> network stack that device is down. Otherwise network stack would >> continue transmitting pakets. >> >> Return value of tsnep_netdev_open() is not checked during BPF program >> setup like in other drivers. Forwarding the return value would result in >> a bpf_prog_put() call in dev_xdp_install(), which would make removal of >> BPF program necessary. >> >> If tsnep_netdev_open() fails during BPF program setup, then the network >> stack would call tsnep_netdev_close() anyway. Thus, tsnep_netdev_close() >> checks now if device is already down. >> >> Additionally remove $(tsnep-y) from $(tsnep-objs) because it is added >> automatically. >> >> Test results with A53 1.2GHz: >> >> XDP_DROP (samples/bpf/xdp1) >> proto 17: 883878 pkt/s >> >> XDP_TX (samples/bpf/xdp2) >> proto 17: 255693 pkt/s >> >> XDP_REDIRECT (samples/bpf/xdpsock) >> sock0@eth2:0 rxdrop xdp-drv >> pps pkts 1.00 >> rx 855,582 5,404,523 >> tx 0 0 >> >> XDP_REDIRECT (samples/bpf/xdp_redirect) >> eth2->eth1 613,267 rx/s 0 err,drop/s 613,272 xmit/s >> >> Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com> >> --- >> drivers/net/ethernet/engleder/Makefile | 2 +- >> drivers/net/ethernet/engleder/tsnep.h | 6 +++++ >> drivers/net/ethernet/engleder/tsnep_main.c | 25 ++++++++++++++++--- >> drivers/net/ethernet/engleder/tsnep_xdp.c | 29 ++++++++++++++++++++++ >> 4 files changed, 58 insertions(+), 4 deletions(-) >> create mode 100644 drivers/net/ethernet/engleder/tsnep_xdp.c >> >> > > <...> > >> --- a/drivers/net/ethernet/engleder/tsnep_main.c >> +++ b/drivers/net/ethernet/engleder/tsnep_main.c >> @@ -1373,7 +1373,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 tx_queue_index = 0; >> @@ -1436,6 +1436,8 @@ static int tsnep_netdev_open(struct net_device *netdev) >> tsnep_enable_irq(adapter, adapter->queue[i].irq_mask); >> } >> >> + netif_tx_start_all_queues(adapter->netdev); >> + >> clear_bit(__TSNEP_DOWN, &adapter->state); >> >> return 0; >> @@ -1457,12 +1459,16 @@ 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; >> >> - set_bit(__TSNEP_DOWN, &adapter->state); >> + if (test_and_set_bit(__TSNEP_DOWN, &adapter->state)) >> + return 0; >> + >> + netif_carrier_off(netdev); >> + netif_tx_stop_all_queues(netdev); >> > > As I called out earlier the __TSNEP_DOWN is just !IFF_UP so you don't > need that bit. > > The fact that netif_carrier_off is here also points out the fact that > the code in the Tx path isn't needed regarding __TSNEP_DOWN and you can > probably just check netif_carrier_ok if you need the check. tsnep_netdev_close() is called directly during bpf prog setup (see tsnep_xdp_setup_prog() in this commit). If the following tsnep_netdev_open() call fails, then this flag signals that the device is already down and nothing needs to be cleaned up if tsnep_netdev_close() is called later (because IFF_UP is still set). Thanks for the review! Gerhard
On Tue, 2023-01-10 at 22:38 +0100, Gerhard Engleder wrote: > On 10.01.23 18:33, Alexander H Duyck wrote: > > On Mon, 2023-01-09 at 20:15 +0100, Gerhard Engleder wrote: > > > Implement setup of BPF programs for XDP RX path with command > > > XDP_SETUP_PROG of ndo_bpf(). This is the final step for XDP RX path > > > support. > > > > > > tsnep_netdev_close() is called directly during BPF program setup. Add > > > netif_carrier_off() and netif_tx_stop_all_queues() calls to signal to > > > network stack that device is down. Otherwise network stack would > > > continue transmitting pakets. > > > > > > Return value of tsnep_netdev_open() is not checked during BPF program > > > setup like in other drivers. Forwarding the return value would result in > > > a bpf_prog_put() call in dev_xdp_install(), which would make removal of > > > BPF program necessary. > > > > > > If tsnep_netdev_open() fails during BPF program setup, then the network > > > stack would call tsnep_netdev_close() anyway. Thus, tsnep_netdev_close() > > > checks now if device is already down. > > > > > > Additionally remove $(tsnep-y) from $(tsnep-objs) because it is added > > > automatically. > > > > > > Test results with A53 1.2GHz: > > > > > > XDP_DROP (samples/bpf/xdp1) > > > proto 17: 883878 pkt/s > > > > > > XDP_TX (samples/bpf/xdp2) > > > proto 17: 255693 pkt/s > > > > > > XDP_REDIRECT (samples/bpf/xdpsock) > > > sock0@eth2:0 rxdrop xdp-drv > > > pps pkts 1.00 > > > rx 855,582 5,404,523 > > > tx 0 0 > > > > > > XDP_REDIRECT (samples/bpf/xdp_redirect) > > > eth2->eth1 613,267 rx/s 0 err,drop/s 613,272 xmit/s > > > > > > Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com> > > > --- > > > drivers/net/ethernet/engleder/Makefile | 2 +- > > > drivers/net/ethernet/engleder/tsnep.h | 6 +++++ > > > drivers/net/ethernet/engleder/tsnep_main.c | 25 ++++++++++++++++--- > > > drivers/net/ethernet/engleder/tsnep_xdp.c | 29 ++++++++++++++++++++++ > > > 4 files changed, 58 insertions(+), 4 deletions(-) > > > create mode 100644 drivers/net/ethernet/engleder/tsnep_xdp.c > > > > > > > > > > <...> > > > > > --- a/drivers/net/ethernet/engleder/tsnep_main.c > > > +++ b/drivers/net/ethernet/engleder/tsnep_main.c > > > @@ -1373,7 +1373,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 tx_queue_index = 0; > > > @@ -1436,6 +1436,8 @@ static int tsnep_netdev_open(struct net_device *netdev) > > > tsnep_enable_irq(adapter, adapter->queue[i].irq_mask); > > > } > > > > > > + netif_tx_start_all_queues(adapter->netdev); > > > + > > > clear_bit(__TSNEP_DOWN, &adapter->state); > > > > > > return 0; > > > @@ -1457,12 +1459,16 @@ 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; > > > > > > - set_bit(__TSNEP_DOWN, &adapter->state); > > > + if (test_and_set_bit(__TSNEP_DOWN, &adapter->state)) > > > + return 0; > > > + > > > + netif_carrier_off(netdev); > > > + netif_tx_stop_all_queues(netdev); > > > > > > > As I called out earlier the __TSNEP_DOWN is just !IFF_UP so you don't > > need that bit. > > > > The fact that netif_carrier_off is here also points out the fact that > > the code in the Tx path isn't needed regarding __TSNEP_DOWN and you can > > probably just check netif_carrier_ok if you need the check. > > tsnep_netdev_close() is called directly during bpf prog setup (see > tsnep_xdp_setup_prog() in this commit). If the following > tsnep_netdev_open() call fails, then this flag signals that the device > is already down and nothing needs to be cleaned up if > tsnep_netdev_close() is called later (because IFF_UP is still set). If the call to close was fouled up you should probably be blocking access to the device via at least a netif_device_detach. I susppose you could use the __LINK_STATE_PRESENT bit as the inverse of the __TSNEP_DOWN bit. If your open fails you clean up, detatch the device, and in the close path you only run through it if the device is present. Basically what we want to avoid is adding a bunch of extra state as what we tend to see is that it will start to create a snarl as you add more and more layers.
On Tue, 10 Jan 2023 22:38:04 +0100 Gerhard Engleder wrote: > > As I called out earlier the __TSNEP_DOWN is just !IFF_UP so you don't > > need that bit. > > > > The fact that netif_carrier_off is here also points out the fact that > > the code in the Tx path isn't needed regarding __TSNEP_DOWN and you can > > probably just check netif_carrier_ok if you need the check. > > tsnep_netdev_close() is called directly during bpf prog setup (see > tsnep_xdp_setup_prog() in this commit). If the following > tsnep_netdev_open() call fails, then this flag signals that the device > is already down and nothing needs to be cleaned up if > tsnep_netdev_close() is called later (because IFF_UP is still set). TBH we've been pushing pretty hard for a while now to stop people from implementing the: close() change config open() sort of reconfiguration. I did that myself when I was a was implementing my first Ethernet driver and DaveM nacked the change. Must have been a decade ago. Imagine you're working on a remote box via SSH and the box is under transient memory pressure. Allocations fail, we don't want the machine to fall off the network :(
On 11.01.23 00:00, Alexander H Duyck wrote: > On Tue, 2023-01-10 at 22:38 +0100, Gerhard Engleder wrote: >> On 10.01.23 18:33, Alexander H Duyck wrote: >>> On Mon, 2023-01-09 at 20:15 +0100, Gerhard Engleder wrote: >>>> Implement setup of BPF programs for XDP RX path with command >>>> XDP_SETUP_PROG of ndo_bpf(). This is the final step for XDP RX path >>>> support. >>>> >>>> tsnep_netdev_close() is called directly during BPF program setup. Add >>>> netif_carrier_off() and netif_tx_stop_all_queues() calls to signal to >>>> network stack that device is down. Otherwise network stack would >>>> continue transmitting pakets. >>>> >>>> Return value of tsnep_netdev_open() is not checked during BPF program >>>> setup like in other drivers. Forwarding the return value would result in >>>> a bpf_prog_put() call in dev_xdp_install(), which would make removal of >>>> BPF program necessary. >>>> >>>> If tsnep_netdev_open() fails during BPF program setup, then the network >>>> stack would call tsnep_netdev_close() anyway. Thus, tsnep_netdev_close() >>>> checks now if device is already down. >>>> >>>> Additionally remove $(tsnep-y) from $(tsnep-objs) because it is added >>>> automatically. <...> >>>> + netif_carrier_off(netdev); >>>> + netif_tx_stop_all_queues(netdev); >>>> >>> >>> As I called out earlier the __TSNEP_DOWN is just !IFF_UP so you don't >>> need that bit. >>> >>> The fact that netif_carrier_off is here also points out the fact that >>> the code in the Tx path isn't needed regarding __TSNEP_DOWN and you can >>> probably just check netif_carrier_ok if you need the check. >> >> tsnep_netdev_close() is called directly during bpf prog setup (see >> tsnep_xdp_setup_prog() in this commit). If the following >> tsnep_netdev_open() call fails, then this flag signals that the device >> is already down and nothing needs to be cleaned up if >> tsnep_netdev_close() is called later (because IFF_UP is still set). > > If the call to close was fouled up you should probably be blocking > access to the device via at least a netif_device_detach. I susppose you > could use the __LINK_STATE_PRESENT bit as the inverse of the > __TSNEP_DOWN bit. If your open fails you clean up, detatch the device, > and in the close path you only run through it if the device is present. > > Basically what we want to avoid is adding a bunch of extra state as > what we tend to see is that it will start to create a snarl as you add > more and more layers. To be honest, I cannot argue that __TSNEP_DOWN is great solution. It is may more about fighting symptoms from the close() change config open() style which according to Jabuk should be prevented anyway. I will suggest a different solution as a reply to Jakubs comment. Gerhard
On 11.01.23 01:12, Jakub Kicinski wrote: > On Tue, 10 Jan 2023 22:38:04 +0100 Gerhard Engleder wrote: >>> As I called out earlier the __TSNEP_DOWN is just !IFF_UP so you don't >>> need that bit. >>> >>> The fact that netif_carrier_off is here also points out the fact that >>> the code in the Tx path isn't needed regarding __TSNEP_DOWN and you can >>> probably just check netif_carrier_ok if you need the check. >> >> tsnep_netdev_close() is called directly during bpf prog setup (see >> tsnep_xdp_setup_prog() in this commit). If the following >> tsnep_netdev_open() call fails, then this flag signals that the device >> is already down and nothing needs to be cleaned up if >> tsnep_netdev_close() is called later (because IFF_UP is still set). > > TBH we've been pushing pretty hard for a while now to stop people > from implementing the: > > close() > change config > open() > > sort of reconfiguration. I did that myself when I was a was > implementing my first Ethernet driver and DaveM nacked the change. > Must have been a decade ago. > > Imagine you're working on a remote box via SSH and the box is under > transient memory pressure. Allocations fail, we don't want the machine > to fall off the network :( I agree with you that this pattern is bad. Most XDP BPF program setup do it like that, but this is of course no valid argument. In the last review round I made the following suggestion (but got no reply so far): What about always using 'XDP_PACKET_HEADROOM' as offset in the RX buffer? The offset 'NET_SKB_PAD + NET_IP_ALIGN' would not even be used if XDP is not enabled. Changing this offset is the only task to be done at the first XDP BFP prog setup call. By always using this offset no close() change config open() pattern is needed. As a result no handling for failed open() is needed and __TSNEP_DOWN is not needed. Simpler code with less problems in my opinion. The only problem could be that NET_IP_ALIGN is not used, but NET_IP_ALIGN is 0 anyway on the two platforms (x86, arm64) where this driver is used. Gerhard
On Wed, 11 Jan 2023 20:11:44 +0100 Gerhard Engleder wrote: > I agree with you that this pattern is bad. Most XDP BPF program setup do > it like that, but this is of course no valid argument. > > In the last review round I made the following suggestion (but got no > reply so far): > > What about always using 'XDP_PACKET_HEADROOM' as offset in the RX > buffer? The offset 'NET_SKB_PAD + NET_IP_ALIGN' would not even be used > if XDP is not enabled. Changing this offset is the only task to be done > at the first XDP BFP prog setup call. By always using this offset > no > > close() > change config > open() > > pattern is needed. As a result no handling for failed open() is needed > and __TSNEP_DOWN is not needed. Simpler code with less problems in my > opinion. > > The only problem could be that NET_IP_ALIGN is not used, but > NET_IP_ALIGN is 0 anyway on the two platforms (x86, arm64) where this > driver is used. You can add NET_IP_ALIGN as well, AFAIU XDP_PACKET_HEADROOM is more of a minimum headroom than an exact one. The other thing off the top of my head is that you'll always need to DMA map the Rx buffers BIDIR. If those are acceptable for your platform / applications then indeed seems like an easy way out of the problem!
diff --git a/drivers/net/ethernet/engleder/Makefile b/drivers/net/ethernet/engleder/Makefile index b6e3b16623de..b98135f65eb7 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-$(CONFIG_TSNEP_SELFTESTS) += tsnep_selftests.o diff --git a/drivers/net/ethernet/engleder/tsnep.h b/drivers/net/ethernet/engleder/tsnep.h index 2268ff793edf..550aae24c8b9 100644 --- a/drivers/net/ethernet/engleder/tsnep.h +++ b/drivers/net/ethernet/engleder/tsnep.h @@ -197,6 +197,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); @@ -220,6 +223,9 @@ 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); + #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 002c879639db..57c35c74dc08 100644 --- a/drivers/net/ethernet/engleder/tsnep_main.c +++ b/drivers/net/ethernet/engleder/tsnep_main.c @@ -1373,7 +1373,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 tx_queue_index = 0; @@ -1436,6 +1436,8 @@ static int tsnep_netdev_open(struct net_device *netdev) tsnep_enable_irq(adapter, adapter->queue[i].irq_mask); } + netif_tx_start_all_queues(adapter->netdev); + clear_bit(__TSNEP_DOWN, &adapter->state); return 0; @@ -1457,12 +1459,16 @@ 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; - set_bit(__TSNEP_DOWN, &adapter->state); + if (test_and_set_bit(__TSNEP_DOWN, &adapter->state)) + return 0; + + netif_carrier_off(netdev); + netif_tx_stop_all_queues(netdev); tsnep_disable_irq(adapter, ECM_INT_LINK); tsnep_phy_close(adapter); @@ -1627,6 +1633,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) { @@ -1677,6 +1695,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..5ced32cd9bb7 --- /dev/null +++ b/drivers/net/ethernet/engleder/tsnep_xdp.c @@ -0,0 +1,29 @@ +// 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; + struct bpf_prog *old_prog; + bool need_reset, running; + + running = netif_running(dev); + need_reset = !!adapter->xdp_prog != !!prog; + if (running && need_reset) + tsnep_netdev_close(dev); + + old_prog = xchg(&adapter->xdp_prog, prog); + if (old_prog) + bpf_prog_put(old_prog); + + if (running && need_reset) + tsnep_netdev_open(dev); + + return 0; +}
Implement setup of BPF programs for XDP RX path with command XDP_SETUP_PROG of ndo_bpf(). This is the final step for XDP RX path support. tsnep_netdev_close() is called directly during BPF program setup. Add netif_carrier_off() and netif_tx_stop_all_queues() calls to signal to network stack that device is down. Otherwise network stack would continue transmitting pakets. Return value of tsnep_netdev_open() is not checked during BPF program setup like in other drivers. Forwarding the return value would result in a bpf_prog_put() call in dev_xdp_install(), which would make removal of BPF program necessary. If tsnep_netdev_open() fails during BPF program setup, then the network stack would call tsnep_netdev_close() anyway. Thus, tsnep_netdev_close() checks now if device is already down. Additionally remove $(tsnep-y) from $(tsnep-objs) because it is added automatically. Test results with A53 1.2GHz: XDP_DROP (samples/bpf/xdp1) proto 17: 883878 pkt/s XDP_TX (samples/bpf/xdp2) proto 17: 255693 pkt/s XDP_REDIRECT (samples/bpf/xdpsock) sock0@eth2:0 rxdrop xdp-drv pps pkts 1.00 rx 855,582 5,404,523 tx 0 0 XDP_REDIRECT (samples/bpf/xdp_redirect) eth2->eth1 613,267 rx/s 0 err,drop/s 613,272 xmit/s Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com> --- drivers/net/ethernet/engleder/Makefile | 2 +- drivers/net/ethernet/engleder/tsnep.h | 6 +++++ drivers/net/ethernet/engleder/tsnep_main.c | 25 ++++++++++++++++--- drivers/net/ethernet/engleder/tsnep_xdp.c | 29 ++++++++++++++++++++++ 4 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 drivers/net/ethernet/engleder/tsnep_xdp.c