| Message ID | 1319033305.8416.29.camel@edumazet-laptop |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 2011-10-19 at 16:08 +0200, Eric Dumazet wrote: > > Anyway, I guess you agree that the patches as-is aren't actually the > > right solution since we can't sock_hold() a TX skb socket reference? > > Yes, the sock_hold() could be changed by an atomic_inc_not_zero() > > What about doing this ? > if (likely(phydev->drv->txtstamp)) { > + if (!atomic_inc_not_zero(&sk->sk_refcnt)) > + return; Yeah that seems like it works and just drops the timestamp in case we don't still have a live socket, which is perfectly fine. johannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 19, 2011 at 04:24:08PM +0200, Johannes Berg wrote: > On Wed, 2011-10-19 at 16:08 +0200, Eric Dumazet wrote: > > > > Anyway, I guess you agree that the patches as-is aren't actually the > > > right solution since we can't sock_hold() a TX skb socket reference? > > > > Yes, the sock_hold() could be changed by an atomic_inc_not_zero() > > > > What about doing this ? > > > if (likely(phydev->drv->txtstamp)) { > > + if (!atomic_inc_not_zero(&sk->sk_refcnt)) > > + return; > > Yeah that seems like it works and just drops the timestamp in case we > don't still have a live socket, which is perfectly fine. Yes, I think it resolves any doubt. I will resubmit with this solution. Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le mercredi 19 octobre 2011 à 16:27 +0200, Richard Cochran a écrit : > On Wed, Oct 19, 2011 at 04:24:08PM +0200, Johannes Berg wrote: > > On Wed, 2011-10-19 at 16:08 +0200, Eric Dumazet wrote: > > > > > > Anyway, I guess you agree that the patches as-is aren't actually the > > > > right solution since we can't sock_hold() a TX skb socket reference? > > > > > > Yes, the sock_hold() could be changed by an atomic_inc_not_zero() > > > > > > What about doing this ? > > > > > if (likely(phydev->drv->txtstamp)) { > > > + if (!atomic_inc_not_zero(&sk->sk_refcnt)) > > > + return; > > > > Yeah that seems like it works and just drops the timestamp in case we > > don't still have a live socket, which is perfectly fine. > > Yes, I think it resolves any doubt. I will resubmit with this > solution. > Thanks guys ! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/phy.h b/include/linux/phy.h index 54fc413..79f337c 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -420,7 +420,7 @@ struct phy_driver { /* * Requests a Tx timestamp for 'skb'. The phy driver promises - * to deliver it to the socket's error queue as soon as a + * to deliver it using skb_complete_tx_timestamp() as soon as a * timestamp becomes available. One of the PTP_CLASS_ values * is passed in 'type'. */ diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 8bd383c..0f96646 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -2020,8 +2020,13 @@ static inline bool skb_defer_rx_timestamp(struct sk_buff *skb) /** * skb_complete_tx_timestamp() - deliver cloned skb with tx timestamps * + * PHY drivers may accept clones of transmitted packets for + * timestamping via their phy_driver.txtstamp method. These drivers + * must call this function to return the skb back to the stack, with + * or without a timestamp. + * * @skb: clone of the the original outgoing packet - * @hwtstamps: hardware time stamps + * @hwtstamps: hardware time stamps, may be NULL if not available * */ void skb_complete_tx_timestamp(struct sk_buff *skb, diff --git a/net/core/timestamping.c b/net/core/timestamping.c index 98a5264..82fb288 100644 --- a/net/core/timestamping.c +++ b/net/core/timestamping.c @@ -57,9 +57,13 @@ void skb_clone_tx_timestamp(struct sk_buff *skb) case PTP_CLASS_V2_VLAN: phydev = skb->dev->phydev; if (likely(phydev->drv->txtstamp)) { + if (!atomic_inc_not_zero(&sk->sk_refcnt)) + return; clone = skb_clone(skb, GFP_ATOMIC); - if (!clone) + if (!clone) { + sock_put(sk); return; + } clone->sk = sk; phydev->drv->txtstamp(phydev, clone, type); } @@ -77,8 +81,11 @@ void skb_complete_tx_timestamp(struct sk_buff *skb, struct sock_exterr_skb *serr; int err; - if (!hwtstamps) + if (!hwtstamps) { + sock_put(sk); + kfree_skb(skb); return; + } *skb_hwtstamps(skb) = *hwtstamps; serr = SKB_EXT_ERR(skb); @@ -87,6 +94,7 @@ void skb_complete_tx_timestamp(struct sk_buff *skb, serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING; skb->sk = NULL; err = sock_queue_err_skb(sk, skb); + sock_put(sk); if (err) kfree_skb(skb); }