From: Wojciech Drewek <[email protected]> Date: Wed, 21 Aug 2024 14:15:37 +0200
> From: Mateusz Polchlopek <[email protected]> > > Rx timestamping introduced in PF driver caused the need of refactoring > the VF driver mechanism to check packet fields. [...] > +static bool iavf_is_descriptor_done(struct iavf_ring *rx_ring, const > + struct iavf_rx_desc *rx_desc) Just pass qw1 here directly instead of @rx_desc. > +{ > + __le64 qw1 = rx_desc->qw1; > + > + if (rx_ring->rxdid == VIRTCHNL_RXDID_1_32B_BASE) I would also pass `bool flex` here instead of @rx_ring. At the beginning of iavf_clean_rx_irq(), do bool flex = rx_ring->rxdid == RXDID_FLEX and then pass this @flex everywhere instead of checking for rx_ring->rxdid. iavf_is_descriptor_done(u64 qw1, bool flex) { if (flex) return le64_get_bits(qw1, FLEX_DD); else return le64_get_bits(qw1, LEGACY_DD); } That's it. > + return !!le64_get_bits(qw1, IAVF_RXD_LEGACY_DD_M); > + > + return !!le64_get_bits(qw1, IAVF_RXD_FLEX_DD_M); > +} > + > static __le64 build_ctob(u32 td_cmd, u32 td_offset, unsigned int size, > u32 td_tag) > { > @@ -1223,24 +1245,31 @@ iavf_extract_legacy_rx_fields(const struct iavf_ring > *rx_ring, > const struct iavf_rx_desc *rx_desc, u32 *ptype) > { > struct libeth_rqe_info fields = {}; > - u64 qw0 = le64_to_cpu(rx_desc->qw0); > u64 qw1 = le64_to_cpu(rx_desc->qw1); > - u64 qw2 = le64_to_cpu(rx_desc->qw2); > - bool l2tag1p; > - bool l2tag2p; > > - fields.eop = FIELD_GET(IAVF_RXD_LEGACY_EOP_M, qw1); > fields.len = FIELD_GET(IAVF_RXD_LEGACY_LENGTH_M, qw1); > - fields.rxe = FIELD_GET(IAVF_RXD_LEGACY_RXE_M, qw1); > - *ptype = FIELD_GET(IAVF_RXD_LEGACY_PTYPE_M, qw1); > - > - l2tag1p = FIELD_GET(IAVF_RXD_LEGACY_L2TAG1P_M, qw1); > - if (l2tag1p && (rx_ring->flags & IAVF_TXRX_FLAGS_VLAN_TAG_LOC_L2TAG1)) > - fields.vlan_tag = FIELD_GET(IAVF_RXD_LEGACY_L2TAG1_M, qw0); > + fields.eop = FIELD_GET(IAVF_RXD_LEGACY_EOP_M, qw1); > > - l2tag2p = FIELD_GET(IAVF_RXD_LEGACY_L2TAG2P_M, qw2); > - if (l2tag2p && (rx_ring->flags & IAVF_RXR_FLAGS_VLAN_TAG_LOC_L2TAG2_2)) > - fields.vlan_tag = FIELD_GET(IAVF_RXD_LEGACY_L2TAG2_M, qw2); > + if (fields.eop) { > + bool l2tag1p, l2tag2p; > + u64 qw0 = le64_to_cpu(rx_desc->qw0); > + u64 qw2 = le64_to_cpu(rx_desc->qw2); > + > + fields.rxe = FIELD_GET(IAVF_RXD_LEGACY_RXE_M, qw1); > + *ptype = FIELD_GET(IAVF_RXD_LEGACY_PTYPE_M, qw1); > + > + l2tag1p = FIELD_GET(IAVF_RXD_LEGACY_L2TAG1P_M, qw1); > + if (l2tag1p && > + (rx_ring->flags & IAVF_TXRX_FLAGS_VLAN_TAG_LOC_L2TAG1)) > + fields.vlan_tag = FIELD_GET(IAVF_RXD_LEGACY_L2TAG1_M, > + qw0); > + > + l2tag2p = FIELD_GET(IAVF_RXD_LEGACY_L2TAG2P_M, qw2); > + if (l2tag2p && > + (rx_ring->flags & IAVF_RXR_FLAGS_VLAN_TAG_LOC_L2TAG2_2)) > + fields.vlan_tag = FIELD_GET(IAVF_RXD_LEGACY_L2TAG2_M, > + qw2); > + } Just do that right where you introduce this function, this change is not related to this patch. Also, `if (!fields.eop) return fields` instead of this big if. > > return fields; > } > @@ -1266,21 +1295,29 @@ iavf_extract_flex_rx_fields(const struct iavf_ring > *rx_ring, > struct libeth_rqe_info fields = {}; > u64 qw0 = le64_to_cpu(rx_desc->qw0); > u64 qw1 = le64_to_cpu(rx_desc->qw1); > - u64 qw2 = le64_to_cpu(rx_desc->qw2); > - bool l2tag1p, l2tag2p; > > fields.len = FIELD_GET(IAVF_RXD_FLEX_PKT_LEN_M, qw0); > - fields.rxe = FIELD_GET(IAVF_RXD_FLEX_RXE_M, qw1); > fields.eop = FIELD_GET(IAVF_RXD_FLEX_EOP_M, qw1); > - *ptype = FIELD_GET(IAVF_RXD_FLEX_PTYPE_M, qw0); > > - l2tag1p = FIELD_GET(IAVF_RXD_FLEX_L2TAG1P_M, qw1); > - if (l2tag1p && (rx_ring->flags & IAVF_TXRX_FLAGS_VLAN_TAG_LOC_L2TAG1)) > - fields.vlan_tag = FIELD_GET(IAVF_RXD_FLEX_L2TAG1_M, qw1); > + if (fields.len) { > + bool l2tag1p, l2tag2p; > + u64 qw2 = le64_to_cpu(rx_desc->qw2); > + > + fields.rxe = FIELD_GET(IAVF_RXD_FLEX_RXE_M, qw1); > + *ptype = FIELD_GET(IAVF_RXD_FLEX_PTYPE_M, qw0); > > - l2tag2p = FIELD_GET(IAVF_RXD_FLEX_L2TAG2P_M, qw2); > - if (l2tag2p && (rx_ring->flags & IAVF_RXR_FLAGS_VLAN_TAG_LOC_L2TAG2_2)) > - fields.vlan_tag = FIELD_GET(IAVF_RXD_FLEX_L2TAG2_2_M, qw2); > + l2tag1p = FIELD_GET(IAVF_RXD_FLEX_L2TAG1P_M, qw1); > + if (l2tag1p && > + (rx_ring->flags & IAVF_TXRX_FLAGS_VLAN_TAG_LOC_L2TAG1)) > + fields.vlan_tag = FIELD_GET(IAVF_RXD_FLEX_L2TAG1_M, > + qw1); > + > + l2tag2p = FIELD_GET(IAVF_RXD_FLEX_L2TAG2P_M, qw2); > + if (l2tag2p && > + (rx_ring->flags & IAVF_RXR_FLAGS_VLAN_TAG_LOC_L2TAG2_2)) > + fields.vlan_tag = FIELD_GET(IAVF_RXD_FLEX_L2TAG2_2_M, > + qw2); > + } Same. > > return fields; > } > @@ -1335,7 +1372,10 @@ static int iavf_clean_rx_irq(struct iavf_ring > *rx_ring, int budget) > */ > dma_rmb(); > > - if (!iavf_test_staterr(rx_desc, IAVF_RXD_FLEX_DD_M)) > + /* If DD field (descriptor done) is unset then other fields are > + * not valid > + */ > + if (!iavf_is_descriptor_done(rx_ring, rx_desc)) > break; > > fields = iavf_extract_rx_fields(rx_ring, rx_desc, &ptype); Thanks, Olek
