From: Claudiu Manoil <claudiu.manoil@gmail.com>
To: Yangbo Lu <yangbo.lu@nxp.com>, netdev@vger.kernel.org
Cc: "David S . Miller" <davem@davemloft.net>,
Richard Cochran <richardcochran@gmail.com>,
Claudiu Manoil <claudiu.manoil@nxp.com>,
Jakub Kicinski <kuba@kernel.org>,
Vladimir Oltean <vladimir.oltean@nxp.com>
Subject: Re: [PATCH 2/2] enetc: support PTP Sync packet one-step timestamping
Date: Sun, 28 Mar 2021 10:51:59 +0300 [thread overview]
Message-ID: <8fa3394e-847f-a3fa-438a-1b357b5726fa@gmail.com> (raw)
In-Reply-To: <20210326083554.28985-3-yangbo.lu@nxp.com>
Hi Yangbo,
Pls add the [net-next] prefix to the subject of these patches next time,
to avoid the patchwork warnings and let reviewers know where to apply them.
On 26.03.2021 10:35, Yangbo Lu wrote:
[...]> +netdev_tx_t enetc_xmit(struct sk_buff *skb, struct net_device *ndev)
> +{
> + struct enetc_ndev_priv *priv = netdev_priv(ndev);
> + u8 udp, msgtype, twostep;
> + u16 offset1, offset2;
> +
> + /* Mark tx timestamp type on skb->cb[0] if requires */
> + if ((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
> + (priv->active_offloads & ENETC_F_TX_TSTAMP_MASK)) {
> + skb->cb[0] = priv->active_offloads & ENETC_F_TX_TSTAMP_MASK;
> + } else {
> + skb->cb[0] = 0;
> + }
> +
> + if (skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
> + /* For one-step PTP sync packet, queue it */
> + if (!enetc_ptp_parse(skb, &udp, &msgtype, &twostep,
> + &offset1, &offset2)) {
> + if (msgtype == PTP_MSGTYPE_SYNC && twostep == 0) {
> + skb_queue_tail(&priv->tx_skbs, skb);
> + queue_work(priv->enetc_ptp_wq,
> + &priv->tx_onestep_tstamp);
> + return NETDEV_TX_OK;
> + }
> + }
> +
> + /* Fall back to two-step timestamp for other packets */
> + skb->cb[0] = ENETC_F_TX_TSTAMP;
> + }
> +
> + return enetc_start_xmit(skb, ndev);
> +}
> +
[...]
> +static void enetc_tx_onestep_tstamp(struct work_struct *work)
> +{
> + struct enetc_ndev_priv *priv;
> + struct sk_buff *skb;
> +
> + priv = container_of(work, struct enetc_ndev_priv, tx_onestep_tstamp);
> +
> + while (true) {
> + skb = skb_dequeue(&priv->tx_skbs);
> + if (!skb)
> + return;
> +
> + /* Lock before TX one-step timestamping packet, and release
> + * when the packet has been sent on hardware, or transmit
> + * failure.
> + */
> + mutex_lock(&priv->onestep_tstamp_lock);
> + enetc_start_xmit(skb, priv->ndev);
> + }
> +}
> +
What happens if the work queue tries to send the ptp packet concurrently
with a regular packet being sent by the stack, via .ndo_start_xmit?
If both skbs are targetting the same tx_ring then we have a concurrency
problem, as enetc_map_tx_buffs(tx_ring, skb) is not thread safe!
Regards,
Claudiu
next prev parent reply other threads:[~2021-03-28 7:52 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-26 8:35 [PATCH 0/2] enetc: support PTP Sync packet one-step timestamping Yangbo Lu
2021-03-26 8:35 ` [PATCH 1/2] enetc: mark TX timestamp type per skb Yangbo Lu
2021-03-26 8:35 ` [PATCH 2/2] enetc: support PTP Sync packet one-step timestamping Yangbo Lu
2021-03-26 12:07 ` kernel test robot
2021-03-28 7:51 ` Claudiu Manoil [this message]
2021-04-08 11:23 ` Y.b. Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8fa3394e-847f-a3fa-438a-1b357b5726fa@gmail.com \
--to=claudiu.manoil@gmail.com \
--cc=claudiu.manoil@nxp.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=richardcochran@gmail.com \
--cc=vladimir.oltean@nxp.com \
--cc=yangbo.lu@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).