[dpdk-dev] [PATCH v5 06/10] net/ixgbe: implement power management API

Wang, Haiyue haiyue.wang at intel.com
Tue Oct 13 03:17:43 CEST 2020


> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov at intel.com>
> Sent: Monday, October 12, 2020 17:29
> To: Wang, Haiyue <haiyue.wang at intel.com>; dev at dpdk.org
> Cc: Ma, Liang J <liang.j.ma at intel.com>; Guo, Jia <jia.guo at intel.com>; Hunt, David
> <david.hunt at intel.com>; Ananyev, Konstantin <konstantin.ananyev at intel.com>; jerinjacobk at gmail.com;
> Richardson, Bruce <bruce.richardson at intel.com>; thomas at monjalon.net; McDaniel, Timothy
> <timothy.mcdaniel at intel.com>; Eads, Gage <gage.eads at intel.com>; Macnamara, Chris
> <chris.macnamara at intel.com>
> Subject: Re: [PATCH v5 06/10] net/ixgbe: implement power management API
> 
> On 12-Oct-20 9:09 AM, Wang, Haiyue wrote:
> > Hi Liang,
> >
> >> -----Original Message-----
> >> From: Burakov, Anatoly <anatoly.burakov at intel.com>
> >> Sent: Saturday, October 10, 2020 00:02
> >> To: dev at dpdk.org
> >> Cc: Ma, Liang J <liang.j.ma at intel.com>; Guo, Jia <jia.guo at intel.com>; Wang, Haiyue
> >> <haiyue.wang at intel.com>; Hunt, David <david.hunt at intel.com>; Ananyev, Konstantin
> >> <konstantin.ananyev at intel.com>; jerinjacobk at gmail.com; Richardson, Bruce
> <bruce.richardson at intel.com>;
> >> thomas at monjalon.net; McDaniel, Timothy <timothy.mcdaniel at intel.com>; Eads, Gage
> <gage.eads at intel.com>;
> >> Macnamara, Chris <chris.macnamara at intel.com>
> >> Subject: [PATCH v5 06/10] net/ixgbe: implement power management API
> >>
> >> From: Liang Ma <liang.j.ma at intel.com>
> >>
> >> Implement support for the power management API by implementing a
> >> `get_wake_addr` function that will return an address of an RX ring's
> >> status bit.
> >>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>
> >> Signed-off-by: Liang Ma <liang.j.ma at intel.com>
> >> ---
> >>   drivers/net/ixgbe/ixgbe_ethdev.c |  1 +
> >>   drivers/net/ixgbe/ixgbe_rxtx.c   | 22 ++++++++++++++++++++++
> >>   drivers/net/ixgbe/ixgbe_rxtx.h   |  2 ++
> >>   3 files changed, 25 insertions(+)
> >>
> >
> >
> >>
> >> +int ixgbe_get_wake_addr(void *rx_queue, volatile void **tail_desc_addr,
> >> +uint64_t *expected, uint64_t *mask)
> >> +{
> >> +volatile union ixgbe_adv_rx_desc *rxdp;
> >> +struct ixgbe_rx_queue *rxq = rx_queue;
> >> +uint16_t desc;
> >> +
> >> +desc = rxq->rx_tail;
> >> +rxdp = &rxq->rx_ring[desc];
> >> +/* watch for changes in status bit */
> >> +*tail_desc_addr = &rxdp->wb.upper.status_error;
> >> +
> >> +/*
> >> + * we expect the DD bit to be set to 1 if this descriptor was already
> >> + * written to.
> >> + */
> >> +*expected = rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD);
> >> +*mask = rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD);
> >> +
> >
> > Seems have one issue about the byte endian:
> > Like for BIG endian:
> >           *expected = rte_bswap32(IXGBE_RXDADV_STAT_DD)
> >               !=
> >           *expected = rte_bswap64(IXGBE_RXDADV_STAT_DD)
> >
> > And in API 'rte_power_monitor', use uint64_t type to access the wake up
> > data:
> >
> > static inline void rte_power_monitor(const volatile void *p,
> > const uint64_t expected_value, const uint64_t value_mask,
> > const uint64_t tsc_timestamp)
> > {
> > if (value_mask) {
> > const uint64_t cur_value = *(const volatile uint64_t *)p;
> > const uint64_t masked = cur_value & value_mask;
> > /* if the masked value is already matching, abort */
> > if (masked == expected_value)
> > return;
> > }
> >
> >
> > So that we need the wake up address type like 16/32/64b ?
> 
> Endian differences strike again! You're right of course.
> 
> I suspect casting everything to CPU endinanness would fix it, would it not?

But need the same date type, if swap is needed for casting, then
(u64 a = rte_bswap32(1)) != (u64 b = rte_bswap64(1))

> 
> >
> >> --
> >> 2.17.1
> 
> 
> --
> Thanks,
> Anatoly


More information about the dev mailing list