Re: [PATCH net] net: fall back to skb_iif for the timestamping pktinfo if_index

JR Lanteigne wrote:
> put_ts_pktinfo() resolves the interface index of a received packet
> from its napi id. The lookup fails on drivers whose napi instances
> are not attached to the delivering net_device (e.g. ti cpsw, which
> keeps them on an internal dummy device) and on kernels built without
> CONFIG_NET_RX_BUSY_POLL, where skb_napi_id() is always 0. In those
> cases SCM_TIMESTAMPING_PKTINFO carries if_index 0 and applications
> cannot tell which interface produced the hardware timestamp. chronyd,
> for example, then ignores valid hardware timestamps.
> 
> Fall back to skb->skb_iif, which is set for every received packet.
> On aggregated interfaces this reports the aggregating device instead
> of the physical one, but only in cases where the napi lookup already
> failed and nothing was reported at all.

Interestingly, the referenced commit explains that it added this
method because the skb_iif reported by IP_PKTINFO is insufficient:

"The index is useful with bonding, bridges and other interfaces, where
IP_PKTINFO doesn't allow applications to determine which PHC made the
timestamp."

I don't mind adding a fallback. As long as possibly passing an
aggregate (or tunnel) device cannot cause regressions to existing
users, notably chrony and linuxptp.
 
> Fixes: aad9c8c470f2 ("net: add new control message for incoming HW-timestamped packets")
> Signed-off-by: JR Lanteigne <[email protected]>
> ---
> A userspace fallback for existing kernels was proposed to chrony
> separately.
> 
>  net/socket.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/socket.c b/net/socket.c
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -900,6 +900,8 @@ static void put_ts_pktinfo(struct msghdr *msg, struct sk_buff *skb,
>  			if_index = orig_dev->ifindex;
>  		rcu_read_unlock();
>  	}
> +	if (!if_index)
> +		if_index = skb->skb_iif;
>  	ts_pktinfo.if_index = if_index;
>  
>  	ts_pktinfo.pkt_length = skb->len - skb_mac_offset(skb);