| Message ID | 20210814140812.46632-3-maciej.fijalkowski@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | XDP_TX improvements for ice | expand |
> -----Original Message----- > From: Fijalkowski, Maciej <maciej.fijalkowski@intel.com> > Sent: Saturday, August 14, 2021 7:08 AM > To: intel-wired-lan@lists.osuosl.org > Cc: netdev@vger.kernel.org; bpf@vger.kernel.org; davem@davemloft.net; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; > kuba@kernel.org; bjorn@kernel.org; Karlsson, Magnus <magnus.karlsson@intel.com>; Brandeburg, Jesse > <jesse.brandeburg@intel.com>; Lobakin, Alexandr <alexandr.lobakin@intel.com>; joamaki@gmail.com; toke@redhat.com; Creeley, > Brett <brett.creeley@intel.com>; Fijalkowski, Maciej <maciej.fijalkowski@intel.com> > Subject: [PATCH v5 intel-next 2/9] ice: move ice_container_type onto ice_ring_container > > Currently ice_container_type is scoped only for ice_ethtool.c. Next > commit that will split the ice_ring struct onto Rx/Tx specific ring > structs is going to also modify the type of linked list of rings that is > within ice_ring_container. Therefore, the functions that are taking the > ice_ring_container as an input argument will need to be aware of a ring > type that will be looked up. > > Embed ice_container_type within ice_ring_container and initialize it > properly when allocating the q_vectors. > > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_base.c | 2 ++ > drivers/net/ethernet/intel/ice/ice_ethtool.c | 36 ++++++++------------ > drivers/net/ethernet/intel/ice/ice_txrx.h | 6 ++++ > 3 files changed, 23 insertions(+), 21 deletions(-) <snip> > +enum ice_container_type { > + ICE_RX_CONTAINER, > + ICE_TX_CONTAINER, > +}; > + > struct ice_ring_container { > /* head of linked-list of rings */ > struct ice_ring *ring; > @@ -347,6 +352,7 @@ struct ice_ring_container { > u16 itr_setting:13; > u16 itr_reserved:2; > u16 itr_mode:1; > + enum ice_container_type type; It may not matter, but should you make sure the size of "type" doesn't negativelly affect this structure? > }; > > struct ice_coalesce_stored { > -- > 2.20.1
On Mon, Aug 16, 2021 at 05:51:06PM +0100, Creeley, Brett wrote: > > -----Original Message----- > > From: Fijalkowski, Maciej <maciej.fijalkowski@intel.com> > > Sent: Saturday, August 14, 2021 7:08 AM > > To: intel-wired-lan@lists.osuosl.org > > Cc: netdev@vger.kernel.org; bpf@vger.kernel.org; davem@davemloft.net; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; > > kuba@kernel.org; bjorn@kernel.org; Karlsson, Magnus <magnus.karlsson@intel.com>; Brandeburg, Jesse > > <jesse.brandeburg@intel.com>; Lobakin, Alexandr <alexandr.lobakin@intel.com>; joamaki@gmail.com; toke@redhat.com; Creeley, > > Brett <brett.creeley@intel.com>; Fijalkowski, Maciej <maciej.fijalkowski@intel.com> > > Subject: [PATCH v5 intel-next 2/9] ice: move ice_container_type onto ice_ring_container > > > > Currently ice_container_type is scoped only for ice_ethtool.c. Next > > commit that will split the ice_ring struct onto Rx/Tx specific ring > > structs is going to also modify the type of linked list of rings that is > > within ice_ring_container. Therefore, the functions that are taking the > > ice_ring_container as an input argument will need to be aware of a ring > > type that will be looked up. > > > > Embed ice_container_type within ice_ring_container and initialize it > > properly when allocating the q_vectors. > > > > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > > --- > > drivers/net/ethernet/intel/ice/ice_base.c | 2 ++ > > drivers/net/ethernet/intel/ice/ice_ethtool.c | 36 ++++++++------------ > > drivers/net/ethernet/intel/ice/ice_txrx.h | 6 ++++ > > 3 files changed, 23 insertions(+), 21 deletions(-) > > <snip> > > > +enum ice_container_type { > > + ICE_RX_CONTAINER, > > + ICE_TX_CONTAINER, > > +}; > > + > > struct ice_ring_container { > > /* head of linked-list of rings */ > > struct ice_ring *ring; > > @@ -347,6 +352,7 @@ struct ice_ring_container { > > u16 itr_setting:13; > > u16 itr_reserved:2; > > u16 itr_mode:1; > > + enum ice_container_type type; > > It may not matter, but should you make sure > the size of "type" doesn't negativelly affect this > structure? Seems that it doesn't matter. Before: struct ice_ring_container { struct ice_ring * ring; /* 0 8 */ struct dim dim; /* 8 120 */ /* XXX last struct has 2 bytes of padding */ /* --- cacheline 2 boundary (128 bytes) --- */ u16 itr_idx; /* 128 2 */ u16 itr_setting:13; /* 130: 0 2 */ u16 itr_reserved:2; /* 130:13 2 */ u16 itr_mode:1; /* 130:15 2 */ /* size: 136, cachelines: 3, members: 6 */ /* padding: 4 */ /* paddings: 1, sum paddings: 2 */ /* last cacheline: 8 bytes */ After: struct ice_ring_container { union { struct ice_rx_ring * rx_ring; /* 0 8 */ struct ice_tx_ring * tx_ring; /* 0 8 */ }; /* 0 8 */ struct dim dim; /* 8 120 */ /* XXX last struct has 2 bytes of padding */ /* --- cacheline 2 boundary (128 bytes) --- */ u16 itr_idx; /* 128 2 */ u16 itr_setting:13; /* 130: 0 2 */ u16 itr_reserved:2; /* 130:13 2 */ u16 itr_mode:1; /* 130:15 2 */ enum ice_container_type type; /* 132 4 */ /* size: 136, cachelines: 3, members: 7 */ /* paddings: 1, sum paddings: 2 */ /* last cacheline: 8 bytes */ Still 3 cachelines and same sizes. > > > }; > > > > struct ice_coalesce_stored { > > -- > > 2.20.1 >
diff --git a/drivers/net/ethernet/intel/ice/ice_base.c b/drivers/net/ethernet/intel/ice/ice_base.c index c36057efc7ae..ab1f88a2af2c 100644 --- a/drivers/net/ethernet/intel/ice/ice_base.c +++ b/drivers/net/ethernet/intel/ice/ice_base.c @@ -115,6 +115,8 @@ static int ice_vsi_alloc_q_vector(struct ice_vsi *vsi, u16 v_idx) q_vector->rx.itr_setting = ICE_DFLT_RX_ITR; q_vector->tx.itr_mode = ITR_DYNAMIC; q_vector->rx.itr_mode = ITR_DYNAMIC; + q_vector->tx.type = ICE_TX_CONTAINER; + q_vector->rx.type = ICE_RX_CONTAINER; if (vsi->type == ICE_VSI_VF) goto out; diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c index d95a5daca114..45b94e4205eb 100644 --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c @@ -3466,11 +3466,6 @@ static int ice_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol) return 0; } -enum ice_container_type { - ICE_RX_CONTAINER, - ICE_TX_CONTAINER, -}; - /** * ice_get_rc_coalesce - get ITR values for specific ring container * @ec: ethtool structure to fill with driver's coalesce settings @@ -3484,13 +3479,12 @@ enum ice_container_type { * Returns 0 on success, negative otherwise. */ static int -ice_get_rc_coalesce(struct ethtool_coalesce *ec, enum ice_container_type c_type, - struct ice_ring_container *rc) +ice_get_rc_coalesce(struct ethtool_coalesce *ec, struct ice_ring_container *rc) { if (!rc->ring) return -EINVAL; - switch (c_type) { + switch (rc->type) { case ICE_RX_CONTAINER: ec->use_adaptive_rx_coalesce = ITR_IS_DYNAMIC(rc); ec->rx_coalesce_usecs = rc->itr_setting; @@ -3501,7 +3495,7 @@ ice_get_rc_coalesce(struct ethtool_coalesce *ec, enum ice_container_type c_type, ec->tx_coalesce_usecs = rc->itr_setting; break; default: - dev_dbg(ice_pf_to_dev(rc->ring->vsi->back), "Invalid c_type %d\n", c_type); + dev_dbg(ice_pf_to_dev(rc->ring->vsi->back), "Invalid c_type %d\n", rc->type); return -EINVAL; } @@ -3522,18 +3516,18 @@ static int ice_get_q_coalesce(struct ice_vsi *vsi, struct ethtool_coalesce *ec, int q_num) { if (q_num < vsi->num_rxq && q_num < vsi->num_txq) { - if (ice_get_rc_coalesce(ec, ICE_RX_CONTAINER, + if (ice_get_rc_coalesce(ec, &vsi->rx_rings[q_num]->q_vector->rx)) return -EINVAL; - if (ice_get_rc_coalesce(ec, ICE_TX_CONTAINER, + if (ice_get_rc_coalesce(ec, &vsi->tx_rings[q_num]->q_vector->tx)) return -EINVAL; } else if (q_num < vsi->num_rxq) { - if (ice_get_rc_coalesce(ec, ICE_RX_CONTAINER, + if (ice_get_rc_coalesce(ec, &vsi->rx_rings[q_num]->q_vector->rx)) return -EINVAL; } else if (q_num < vsi->num_txq) { - if (ice_get_rc_coalesce(ec, ICE_TX_CONTAINER, + if (ice_get_rc_coalesce(ec, &vsi->tx_rings[q_num]->q_vector->tx)) return -EINVAL; } else { @@ -3595,10 +3589,10 @@ ice_get_per_q_coalesce(struct net_device *netdev, u32 q_num, * Returns 0 on success, negative otherwise. */ static int -ice_set_rc_coalesce(enum ice_container_type c_type, struct ethtool_coalesce *ec, +ice_set_rc_coalesce(struct ethtool_coalesce *ec, struct ice_ring_container *rc, struct ice_vsi *vsi) { - const char *c_type_str = (c_type == ICE_RX_CONTAINER) ? "rx" : "tx"; + const char *c_type_str = (rc->type == ICE_RX_CONTAINER) ? "rx" : "tx"; u32 use_adaptive_coalesce, coalesce_usecs; struct ice_pf *pf = vsi->back; u16 itr_setting; @@ -3606,7 +3600,7 @@ ice_set_rc_coalesce(enum ice_container_type c_type, struct ethtool_coalesce *ec, if (!rc->ring) return -EINVAL; - switch (c_type) { + switch (rc->type) { case ICE_RX_CONTAINER: if (ec->rx_coalesce_usecs_high > ICE_MAX_INTRL || (ec->rx_coalesce_usecs_high && @@ -3639,7 +3633,7 @@ ice_set_rc_coalesce(enum ice_container_type c_type, struct ethtool_coalesce *ec, break; default: dev_dbg(ice_pf_to_dev(pf), "Invalid container type %d\n", - c_type); + rc->type); return -EINVAL; } @@ -3688,22 +3682,22 @@ static int ice_set_q_coalesce(struct ice_vsi *vsi, struct ethtool_coalesce *ec, int q_num) { if (q_num < vsi->num_rxq && q_num < vsi->num_txq) { - if (ice_set_rc_coalesce(ICE_RX_CONTAINER, ec, + if (ice_set_rc_coalesce(ec, &vsi->rx_rings[q_num]->q_vector->rx, vsi)) return -EINVAL; - if (ice_set_rc_coalesce(ICE_TX_CONTAINER, ec, + if (ice_set_rc_coalesce(ec, &vsi->tx_rings[q_num]->q_vector->tx, vsi)) return -EINVAL; } else if (q_num < vsi->num_rxq) { - if (ice_set_rc_coalesce(ICE_RX_CONTAINER, ec, + if (ice_set_rc_coalesce(ec, &vsi->rx_rings[q_num]->q_vector->rx, vsi)) return -EINVAL; } else if (q_num < vsi->num_txq) { - if (ice_set_rc_coalesce(ICE_TX_CONTAINER, ec, + if (ice_set_rc_coalesce(ec, &vsi->tx_rings[q_num]->q_vector->tx, vsi)) return -EINVAL; diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h b/drivers/net/ethernet/intel/ice/ice_txrx.h index d2fd90607b70..270c00536eee 100644 --- a/drivers/net/ethernet/intel/ice/ice_txrx.h +++ b/drivers/net/ethernet/intel/ice/ice_txrx.h @@ -336,6 +336,11 @@ static inline bool ice_ring_is_xdp(struct ice_ring *ring) return !!(ring->flags & ICE_TX_FLAGS_RING_XDP); } +enum ice_container_type { + ICE_RX_CONTAINER, + ICE_TX_CONTAINER, +}; + struct ice_ring_container { /* head of linked-list of rings */ struct ice_ring *ring; @@ -347,6 +352,7 @@ struct ice_ring_container { u16 itr_setting:13; u16 itr_reserved:2; u16 itr_mode:1; + enum ice_container_type type; }; struct ice_coalesce_stored {
Currently ice_container_type is scoped only for ice_ethtool.c. Next commit that will split the ice_ring struct onto Rx/Tx specific ring structs is going to also modify the type of linked list of rings that is within ice_ring_container. Therefore, the functions that are taking the ice_ring_container as an input argument will need to be aware of a ring type that will be looked up. Embed ice_container_type within ice_ring_container and initialize it properly when allocating the q_vectors. Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> --- drivers/net/ethernet/intel/ice/ice_base.c | 2 ++ drivers/net/ethernet/intel/ice/ice_ethtool.c | 36 ++++++++------------ drivers/net/ethernet/intel/ice/ice_txrx.h | 6 ++++ 3 files changed, 23 insertions(+), 21 deletions(-)