[Intel-wired-lan] [PATCH v4 7/9] igc: Add initial XDP support
Neftin, Sasha
sasha.neftin at intel.com
Thu Nov 12 09:13:27 UTC 2020
On 11/4/2020 05:12, Andre Guedes wrote:
> This patch adds the initial XDP support to the igc driver. For now,
> only XDP_PASS, XDP_DROP, XDP_ABORTED actions are supported. Upcoming
> patches will add support for the remaining XDP actions.
>
> XDP configuration helpers are defined in a new file, igc_xdp.c. These
> helpers are utilized in igc_main.c to implement the ndo_bpf callback.
> XDP-related code that belongs to the driver's hot path is landed in
> igc_main.c.
>
> By default, the driver uses rx buffers with 2 KB size. When XDP is
> enabled, it uses larger buffers so we have enough space to accommodate
> the headroom and tailroom required by XDP infrastructure. Also, the
> driver doesn't support XDP functionality with frames that span over
> multiple buffers so jumbo frames are not allowed for now.
>
> The approach implemented by this patch follows the approach implemented
> in other Intel drivers as much as possible for the sake of consistency
> across the drivers.
>
> Quick comment regarding igc_build_skb(): this patch doesn't touch it
> because the function is never called. It seems its support is
> incomplete/in progress. The function was added by commit 0507ef8a0372b
> ("igc: Add transmit and receive fastpath and interrupt handlers") but
> ring_uses_build_skb() always return False since the IGC_RING_FLAG_RX_
> BUILD_SKB_ENABLED isn't set anywhere in the driver code.
>
> This patch has been tested with the sample app "xdp1" located in
> samples/bpf/ dir.
>
> Signed-off-by: Andre Guedes <andre.guedes at intel.com>
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski at intel.com>
> ---
> drivers/net/ethernet/intel/igc/Makefile | 2 +-
> drivers/net/ethernet/intel/igc/igc.h | 2 +
> drivers/net/ethernet/intel/igc/igc_main.c | 118 ++++++++++++++++++++--
> drivers/net/ethernet/intel/igc/igc_xdp.c | 33 ++++++
> drivers/net/ethernet/intel/igc/igc_xdp.h | 10 ++
> 5 files changed, 153 insertions(+), 12 deletions(-)
> create mode 100644 drivers/net/ethernet/intel/igc/igc_xdp.c
> create mode 100644 drivers/net/ethernet/intel/igc/igc_xdp.h
>
> diff --git a/drivers/net/ethernet/intel/igc/Makefile b/drivers/net/ethernet/intel/igc/Makefile
> index 1c3051db9085..95d1e8c490a4 100644
> --- a/drivers/net/ethernet/intel/igc/Makefile
> +++ b/drivers/net/ethernet/intel/igc/Makefile
> @@ -8,4 +8,4 @@
> obj-$(CONFIG_IGC) += igc.o
>
> igc-objs := igc_main.o igc_mac.o igc_i225.o igc_base.o igc_nvm.o igc_phy.o \
> -igc_diag.o igc_ethtool.o igc_ptp.o igc_dump.o igc_tsn.o
> +igc_diag.o igc_ethtool.o igc_ptp.o igc_dump.o igc_tsn.o igc_xdp.o
> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
> index e72f1fc772aa..5c2f363106ae 100644
> --- a/drivers/net/ethernet/intel/igc/igc.h
> +++ b/drivers/net/ethernet/intel/igc/igc.h
> @@ -224,6 +224,8 @@ struct igc_adapter {
> struct mutex ptm_time_lock; /* protects host and device timestamps */
> ktime_t ptm_device_time;
> struct system_counterval_t ptm_host_time;
> +
> + struct bpf_prog *xdp_prog;
> };
>
> void igc_up(struct igc_adapter *adapter);
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 84ffde75e968..734a570bbadb 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -11,17 +11,22 @@
> #include <linux/pm_runtime.h>
> #include <net/pkt_sched.h>
> #include <linux/pci.h>
> +#include <linux/bpf_trace.h>
>
> #include <net/ipv6.h>
>
> #include "igc.h"
> #include "igc_hw.h"
> #include "igc_tsn.h"
> +#include "igc_xdp.h"
>
> #define DRV_SUMMARY "Intel(R) 2.5G Ethernet Linux Driver"
>
> #define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_LINK)
>
> +#define IGC_XDP_PASS 0
> +#define IGC_XDP_CONSUMED BIT(0)
> +
Hello Andre, please, move these defines to igc_defines.h to bottom the
file under /* XDP defines */ comment.
> static int debug = -1;
>
> MODULE_AUTHOR("Intel Corporation, <linux.nics at intel.com>");
> @@ -346,6 +351,8 @@ static void igc_clean_rx_ring(struct igc_ring *rx_ring)
> {
> u16 i = rx_ring->next_to_clean;
>
> + clear_ring_uses_large_buffer(rx_ring);
> +
> dev_kfree_skb(rx_ring->skb);
> rx_ring->skb = NULL;
>
> @@ -498,6 +505,11 @@ static int igc_setup_all_rx_resources(struct igc_adapter *adapter)
> return err;
> }
>
> +static bool igc_xdp_is_enabled(struct igc_adapter *adapter)
> +{
> + return !!adapter->xdp_prog;
> +}
> +
> /**
> * igc_configure_rx_ring - Configure a receive ring after Reset
> * @adapter: board private structure
> @@ -514,6 +526,9 @@ static void igc_configure_rx_ring(struct igc_adapter *adapter,
> u32 srrctl = 0, rxdctl = 0;
> u64 rdba = ring->dma;
>
> + if (igc_xdp_is_enabled(adapter))
> + set_ring_uses_large_buffer(ring);
> +
> /* disable the queue */
> wr32(IGC_RXDCTL(reg_idx), 0);
>
> @@ -1594,12 +1609,12 @@ static struct sk_buff *igc_build_skb(struct igc_ring *rx_ring,
>
> static struct sk_buff *igc_construct_skb(struct igc_ring *rx_ring,
> struct igc_rx_buffer *rx_buffer,
> - unsigned int size, int pkt_offset,
> + struct xdp_buff *xdp,
> ktime_t timestamp)
> {
> - void *va = page_address(rx_buffer->page) + rx_buffer->page_offset +
> - pkt_offset;
> + unsigned int size = xdp->data_end - xdp->data;
> unsigned int truesize = igc_get_rx_frame_truesize(rx_ring, size);
> + void *va = xdp->data;
> unsigned int headlen;
> struct sk_buff *skb;
>
> @@ -1748,6 +1763,10 @@ static bool igc_cleanup_headers(struct igc_ring *rx_ring,
> union igc_adv_rx_desc *rx_desc,
> struct sk_buff *skb)
> {
> + /* XDP packets use error pointer so abort at this point */
> + if (IS_ERR(skb))
> + return true;
> +
> if (unlikely(igc_test_staterr(rx_desc, IGC_RXDEXT_STATERR_RXE))) {
> struct net_device *netdev = rx_ring->netdev;
>
> @@ -1787,7 +1806,14 @@ static void igc_put_rx_buffer(struct igc_ring *rx_ring,
>
> static inline unsigned int igc_rx_offset(struct igc_ring *rx_ring)
> {
> - return ring_uses_build_skb(rx_ring) ? IGC_SKB_PAD : 0;
> + struct igc_adapter *adapter = rx_ring->q_vector->adapter;
> +
> + if (ring_uses_build_skb(rx_ring))
> + return IGC_SKB_PAD;
> + if (igc_xdp_is_enabled(adapter))
> + return XDP_PACKET_HEADROOM;
> +
> + return 0;
> }
>
> static bool igc_alloc_mapped_page(struct igc_ring *rx_ring,
> @@ -1901,6 +1927,42 @@ static void igc_alloc_rx_buffers(struct igc_ring *rx_ring, u16 cleaned_count)
> }
> }
>
> +static struct sk_buff *igc_xdp_run_prog(struct igc_adapter *adapter,
> + struct xdp_buff *xdp)
> +{
> + struct bpf_prog *prog;
> + int res;
> + u32 act;
> +
> + rcu_read_lock();
> +
> + prog = READ_ONCE(adapter->xdp_prog);
> + if (!prog) {
> + res = IGC_XDP_PASS;
> + goto unlock;
> + }
> +
> + act = bpf_prog_run_xdp(prog, xdp);
> + switch (act) {
> + case XDP_PASS:
> + res = IGC_XDP_PASS;
> + break;
> + default:
> + bpf_warn_invalid_xdp_action(act);
> + fallthrough;
> + case XDP_ABORTED:
> + trace_xdp_exception(adapter->netdev, prog, act);
> + fallthrough;
> + case XDP_DROP:
> + res = IGC_XDP_CONSUMED;
> + break;
> + }
> +
> +unlock:
> + rcu_read_unlock();
> + return ERR_PTR(-res);
> +}
> +
> static int igc_clean_rx_irq(struct igc_q_vector *q_vector, const int budget)
> {
> unsigned int total_bytes = 0, total_packets = 0;
> @@ -1912,8 +1974,10 @@ static int igc_clean_rx_irq(struct igc_q_vector *q_vector, const int budget)
> union igc_adv_rx_desc *rx_desc;
> struct igc_rx_buffer *rx_buffer;
> ktime_t timestamp = 0;
> + struct xdp_buff xdp;
> int pkt_offset = 0;
> unsigned int size;
> + void *pktbuf;
>
> /* return some buffers to hardware, one at a time is too slow */
> if (cleaned_count >= IGC_RX_BUFFER_WRITE) {
> @@ -1934,24 +1998,38 @@ static int igc_clean_rx_irq(struct igc_q_vector *q_vector, const int budget)
>
> rx_buffer = igc_get_rx_buffer(rx_ring, size);
>
> - if (igc_test_staterr(rx_desc, IGC_RXDADV_STAT_TSIP)) {
> - void *pktbuf = page_address(rx_buffer->page) +
> - rx_buffer->page_offset;
> + pktbuf = page_address(rx_buffer->page) + rx_buffer->page_offset;
>
> + if (igc_test_staterr(rx_desc, IGC_RXDADV_STAT_TSIP)) {
> timestamp = igc_ptp_rx_pktstamp(q_vector->adapter,
> pktbuf);
> pkt_offset = IGC_TS_HDR_LEN;
> size -= IGC_TS_HDR_LEN;
> }
>
> - /* retrieve a buffer from the ring */
> - if (skb)
> + if (!skb) {
> + struct igc_adapter *adapter = q_vector->adapter;
> +
> + xdp.data = pktbuf + pkt_offset;
> + xdp.data_end = xdp.data + size;
> + xdp.data_hard_start = pktbuf - igc_rx_offset(rx_ring);
> + xdp_set_data_meta_invalid(&xdp);
> + xdp.frame_sz = igc_get_rx_frame_truesize(rx_ring, size);
> +
> + skb = igc_xdp_run_prog(adapter, &xdp);
> + }
> +
> + if (IS_ERR(skb)) {
> + rx_buffer->pagecnt_bias++;
> + total_packets++;
> + total_bytes += size;
> + } else if (skb)
> igc_add_rx_frag(rx_ring, rx_buffer, skb, size);
> else if (ring_uses_build_skb(rx_ring))
> skb = igc_build_skb(rx_ring, rx_buffer, rx_desc, size);
> else
> - skb = igc_construct_skb(rx_ring, rx_buffer, size,
> - pkt_offset, timestamp);
> + skb = igc_construct_skb(rx_ring, rx_buffer, &xdp,
> + timestamp);
>
> /* exit if we failed to retrieve a buffer */
> if (!skb) {
> @@ -3893,6 +3971,11 @@ static int igc_change_mtu(struct net_device *netdev, int new_mtu)
> int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
> struct igc_adapter *adapter = netdev_priv(netdev);
>
> + if (igc_xdp_is_enabled(adapter) && new_mtu > ETH_DATA_LEN) {
> + netdev_dbg(netdev, "Jumbo frames not supported with XDP");
> + return -EINVAL;
> + }
> +
> /* adjust max frame to be at least the size of a standard frame */
> if (max_frame < (ETH_FRAME_LEN + ETH_FCS_LEN))
> max_frame = ETH_FRAME_LEN + ETH_FCS_LEN;
> @@ -4881,6 +4964,18 @@ static int igc_setup_tc(struct net_device *dev, enum tc_setup_type type,
> }
> }
>
> +static int igc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
> +{
> + struct igc_adapter *adapter = netdev_priv(dev);
> +
> + switch (bpf->command) {
> + case XDP_SETUP_PROG:
> + return igc_xdp_set_prog(adapter, bpf->prog, bpf->extack);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> static const struct net_device_ops igc_netdev_ops = {
> .ndo_open = igc_open,
> .ndo_stop = igc_close,
> @@ -4894,6 +4989,7 @@ static const struct net_device_ops igc_netdev_ops = {
> .ndo_features_check = igc_features_check,
> .ndo_do_ioctl = igc_ioctl,
> .ndo_setup_tc = igc_setup_tc,
> + .ndo_bpf = igc_bpf,
> };
>
> /* PCIe configuration access */
> diff --git a/drivers/net/ethernet/intel/igc/igc_xdp.c b/drivers/net/ethernet/intel/igc/igc_xdp.c
> new file mode 100644
> index 000000000000..27c886a254f1
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/igc/igc_xdp.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2020, Intel Corporation. */
> +
> +#include "igc.h"
> +#include "igc_xdp.h"
> +
> +int igc_xdp_set_prog(struct igc_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 (dev->mtu > ETH_DATA_LEN) {
> + /* For now, the driver doesn't support XDP functionality with
> + * jumbo frames so we return error.
> + */
> + NL_SET_ERR_MSG_MOD(extack, "Jumbo frames not supported");
> + return -EOPNOTSUPP;
> + }
> +
> + if (if_running)
> + igc_close(dev);
> +
> + old_prog = xchg(&adapter->xdp_prog, prog);
> + if (old_prog)
> + bpf_prog_put(old_prog);
> +
> + if (if_running)
> + igc_open(dev);
> +
> + return 0;
> +}
> diff --git a/drivers/net/ethernet/intel/igc/igc_xdp.h b/drivers/net/ethernet/intel/igc/igc_xdp.h
> new file mode 100644
> index 000000000000..8a410bcefe1a
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/igc/igc_xdp.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2020, Intel Corporation. */
> +
> +#ifndef _IGC_XDP_H_
> +#define _IGC_XDP_H_
> +
> +int igc_xdp_set_prog(struct igc_adapter *adapter, struct bpf_prog *prog,
> + struct netlink_ext_ack *extack);
> +
> +#endif /* _IGC_XDP_H_ */
>
Thanks,
Sasha
More information about the Intel-wired-lan
mailing list