| Message ID | 152775719796.24817.11035788244128769860.stgit@firesoul |
|---|---|
| State | Accepted, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf/xdp: add flags argument to ndo_xdp_xmit and flag flush operation | expand |
On Thu, May 31, 2018 at 1:59 AM, Jesper Dangaard Brouer <brouer@redhat.com> wrote: > When passed the XDP_XMIT_FLUSH flag ixgbe_xdp_xmit now performs the > same kind of ring tail update as in ixgbe_xdp_flush. The update tail > code in ixgbe_xdp_flush is generalized and shared with ixgbe_xdp_xmit. > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> Acked-by: Song Liu <songliubraving@fb.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > index 87f088f4af52..4fd77c9067f2 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > @@ -10022,6 +10022,15 @@ static int ixgbe_xdp(struct net_device *dev, struct netdev_bpf *xdp) > } > } > > +static void ixgbe_xdp_ring_update_tail(struct ixgbe_ring *ring) > +{ > + /* Force memory writes to complete before letting h/w know there > + * are new descriptors to fetch. > + */ > + wmb(); > + writel(ring->next_to_use, ring->tail); > +} > + > static int ixgbe_xdp_xmit(struct net_device *dev, int n, > struct xdp_frame **frames, u32 flags) > { > @@ -10033,7 +10042,7 @@ static int ixgbe_xdp_xmit(struct net_device *dev, int n, > if (unlikely(test_bit(__IXGBE_DOWN, &adapter->state))) > return -ENETDOWN; > > - if (unlikely(flags & ~XDP_XMIT_FLAGS_NONE)) > + if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) > return -EINVAL; > > /* During program transitions its possible adapter->xdp_prog is assigned > @@ -10054,6 +10063,9 @@ static int ixgbe_xdp_xmit(struct net_device *dev, int n, > } > } > > + if (unlikely(flags & XDP_XMIT_FLUSH)) > + ixgbe_xdp_ring_update_tail(ring); > + > return n - drops; > } > > @@ -10072,11 +10084,7 @@ static void ixgbe_xdp_flush(struct net_device *dev) > if (unlikely(!ring)) > return; > > - /* Force memory writes to complete before letting h/w know there > - * are new descriptors to fetch. > - */ > - wmb(); > - writel(ring->next_to_use, ring->tail); > + ixgbe_xdp_ring_update_tail(ring); > > return; > } >
On 05/31/2018 10:59 AM, Jesper Dangaard Brouer wrote: > When passed the XDP_XMIT_FLUSH flag ixgbe_xdp_xmit now performs the > same kind of ring tail update as in ixgbe_xdp_flush. The update tail > code in ixgbe_xdp_flush is generalized and shared with ixgbe_xdp_xmit. > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > index 87f088f4af52..4fd77c9067f2 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > @@ -10022,6 +10022,15 @@ static int ixgbe_xdp(struct net_device *dev, struct netdev_bpf *xdp) > } > } > > +static void ixgbe_xdp_ring_update_tail(struct ixgbe_ring *ring) > +{ > + /* Force memory writes to complete before letting h/w know there > + * are new descriptors to fetch. > + */ > + wmb(); > + writel(ring->next_to_use, ring->tail); > +} Did you double check that this doesn't become a function call? Should this get an __always_inline attribute? > + > static int ixgbe_xdp_xmit(struct net_device *dev, int n, > struct xdp_frame **frames, u32 flags) > { > @@ -10033,7 +10042,7 @@ static int ixgbe_xdp_xmit(struct net_device *dev, int n, > if (unlikely(test_bit(__IXGBE_DOWN, &adapter->state))) > return -ENETDOWN; > > - if (unlikely(flags & ~XDP_XMIT_FLAGS_NONE)) > + if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) > return -EINVAL; > > /* During program transitions its possible adapter->xdp_prog is assigned > @@ -10054,6 +10063,9 @@ static int ixgbe_xdp_xmit(struct net_device *dev, int n, > } > } > > + if (unlikely(flags & XDP_XMIT_FLUSH)) > + ixgbe_xdp_ring_update_tail(ring); > + > return n - drops; > } > > @@ -10072,11 +10084,7 @@ static void ixgbe_xdp_flush(struct net_device *dev) > if (unlikely(!ring)) > return; > > - /* Force memory writes to complete before letting h/w know there > - * are new descriptors to fetch. > - */ > - wmb(); > - writel(ring->next_to_use, ring->tail); > + ixgbe_xdp_ring_update_tail(ring); > > return; > } >
On Mon, 4 Jun 2018 15:19:05 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote: > > +static void ixgbe_xdp_ring_update_tail(struct ixgbe_ring *ring) > > +{ > > + /* Force memory writes to complete before letting h/w know there > > + * are new descriptors to fetch. > > + */ > > + wmb(); > > + writel(ring->next_to_use, ring->tail); > > +} > > Did you double check that this doesn't become a function call? Should this > get an __always_inline attribute? I did check this doesn't become a function call. The same kind of code happens other places in the driver, but I choose not to generalize this, exactly to avoid this becoming a function call ;-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 87f088f4af52..4fd77c9067f2 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -10022,6 +10022,15 @@ static int ixgbe_xdp(struct net_device *dev, struct netdev_bpf *xdp) } } +static void ixgbe_xdp_ring_update_tail(struct ixgbe_ring *ring) +{ + /* Force memory writes to complete before letting h/w know there + * are new descriptors to fetch. + */ + wmb(); + writel(ring->next_to_use, ring->tail); +} + static int ixgbe_xdp_xmit(struct net_device *dev, int n, struct xdp_frame **frames, u32 flags) { @@ -10033,7 +10042,7 @@ static int ixgbe_xdp_xmit(struct net_device *dev, int n, if (unlikely(test_bit(__IXGBE_DOWN, &adapter->state))) return -ENETDOWN; - if (unlikely(flags & ~XDP_XMIT_FLAGS_NONE)) + if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) return -EINVAL; /* During program transitions its possible adapter->xdp_prog is assigned @@ -10054,6 +10063,9 @@ static int ixgbe_xdp_xmit(struct net_device *dev, int n, } } + if (unlikely(flags & XDP_XMIT_FLUSH)) + ixgbe_xdp_ring_update_tail(ring); + return n - drops; } @@ -10072,11 +10084,7 @@ static void ixgbe_xdp_flush(struct net_device *dev) if (unlikely(!ring)) return; - /* Force memory writes to complete before letting h/w know there - * are new descriptors to fetch. - */ - wmb(); - writel(ring->next_to_use, ring->tail); + ixgbe_xdp_ring_update_tail(ring); return; }
When passed the XDP_XMIT_FLUSH flag ixgbe_xdp_xmit now performs the same kind of ring tail update as in ixgbe_xdp_flush. The update tail code in ixgbe_xdp_flush is generalized and shared with ixgbe_xdp_xmit. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)