lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 15 Oct 2019 20:07:49 +0000 From: Ioana Ciornei <ioana.ciornei@....com> To: Andrew Lunn <andrew@...n.ch>, Jakub Kicinski <jakub.kicinski@...ronome.com> CC: "davem@...emloft.net" <davem@...emloft.net>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Ioana Ciocoi Radulescu <ruxandra.radulescu@....com> Subject: Re: [PATCH v2 net 2/2] dpaa2-eth: Fix TX FQID values On 10/15/19 10:47 PM, Andrew Lunn wrote: > On Tue, Oct 15, 2019 at 12:40:17PM -0700, Jakub Kicinski wrote: >> On Tue, 15 Oct 2019 21:29:23 +0200, Andrew Lunn wrote: >>>> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c >>>> index 5acd734a216b..c3c2c06195ae 100644 >>>> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c >>>> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c >>>> @@ -1235,6 +1235,8 @@ static void dpaa2_eth_set_rx_taildrop(struct dpaa2_eth_priv *priv, bool enable) >>>> priv->rx_td_enabled = enable; >>>> } >>>> >>>> +static void update_tx_fqids(struct dpaa2_eth_priv *priv); >>>> + >>> >>> Hi Ioana and Ioana >>> >>> Using forward declarations is generally not liked. Is there something >>> which is preventing you from having it earlier in the file? >> >> Ha! I was just about to ask the same question 😊 >> >> +out_err: >> + netdev_info(priv->net_dev, >> + "Error reading Tx FQID, fallback to QDID-based enqueue"); >> + priv->enqueue = dpaa2_eth_enqueue_qd; >> +} >> >> Here dpaa2_eth_enqueue_qd is a function pointer which is is defined >> towards the end of the file :S > > Hi Jakub > > Thanks, i was too lazy to look. > > So this is O.K. for net, since fixes should be minimal. > > Ioana's please could you submit a patch to net-next, once this has > been merged, to move the code around to remove the forward > declarations. Yes, I'll cleanup this in net-next. Also, no more Ioana's for a while, just me :) > >> Also can I point out that this: >> >> static inline int dpaa2_eth_enqueue_qd(struct dpaa2_eth_priv *priv, >> struct dpaa2_eth_fq *fq, >> struct dpaa2_fd *fd, u8 prio) >> { >> return dpaa2_io_service_enqueue_qd(fq->channel->dpio, >> priv->tx_qdid, prio, >> fq->tx_qdbin, fd); >> } >> >> static inline int dpaa2_eth_enqueue_fq(struct dpaa2_eth_priv *priv, >> struct dpaa2_eth_fq *fq, >> struct dpaa2_fd *fd, u8 prio) >> { >> return dpaa2_io_service_enqueue_fq(fq->channel->dpio, >> fq->tx_fqid[prio], fd); >> } >> >> static void set_enqueue_mode(struct dpaa2_eth_priv *priv) >> { >> if (dpaa2_eth_cmp_dpni_ver(priv, DPNI_ENQUEUE_FQID_VER_MAJOR, >> DPNI_ENQUEUE_FQID_VER_MINOR) < 0) >> priv->enqueue = dpaa2_eth_enqueue_qd; >> else >> priv->enqueue = dpaa2_eth_enqueue_fq; >> } >> >> Could be the most pointless use of the inline keyword possible :) >> Both dpaa2_eth_enqueue_qd() and dpaa2_eth_enqueue_fq() are only ever >> called via a pointer.. I think that's another change for net-next. > > Is priv->enqueue used on the hotpath? SPECTRA/Meltdown etc make that > expensive. > Yes, the priv->enqueue is used on the Tx path to enqueue a skb (transformed into a frame descriptor) into a queue. I need to benchmark if transforming the callback into an if-else statement is helping. Ioana C > Andrew >
Powered by blists - more mailing lists