netdev - Re: [PATCH net-next-2.6] net: force a fresh timestamp for ingress packets

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20101216211744.GA2191@del.dom.local>
Date:	Thu, 16 Dec 2010 22:17:45 +0100
From:	Jarek Poplawski <jarkao2@...il.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	David Miller <davem@...emloft.net>,
	Changli Gao <xiaosuo@...il.com>,
	netdev <netdev@...r.kernel.org>,
	Patrick McHardy <kaber@...sh.net>
Subject: Re: [PATCH net-next-2.6] net: force a fresh timestamp for ingress
 packets

On Wed, Dec 15, 2010 at 04:50:52PM +0100, Eric Dumazet wrote:
> In commit 8caf153974f2 (net: sch_netem: Fix an inconsistency in ingress
> netem timestamps.), Jarek added a logic to refresh timestamps of
> ingressed packets going through netem.
> 
> I believe we should generalize this, forcing a refresh of timestamps in
> dev_queue_xmit_nit() for all ingress packets, whatever qdisc/class they
> used before being delivered.
> 
> This way, we can have a good idea when packets are delivered to our
> stack (tcpdump -i ifb0), while a tcpdump on original device gives
> timestamps right before ingressing.

I don't think we should do it. IMHO netem on ingress is a special case,
obviously for testing, and otherwise the real (first) timestamp might
be valuable for some users.

Jarek P.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
> ---
>  net/core/dev.c        |    2 +-
>  net/sched/sch_netem.c |    8 --------
>  2 files changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index d28b3a0..a2846f8 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1506,7 +1506,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
>  	struct packet_type *ptype;
>  
>  #ifdef CONFIG_NET_CLS_ACT
> -	if (!(skb->tstamp.tv64 && (G_TC_FROM(skb->tc_verd) & AT_INGRESS)))
> +	if (!skb->tstamp.tv64 || (G_TC_FROM(skb->tc_verd) & AT_INGRESS))
>  		net_timestamp_set(skb);
>  #else
>  	net_timestamp_set(skb);
> diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
> index e5593c0..1c29cc0 100644
> --- a/net/sched/sch_netem.c
> +++ b/net/sched/sch_netem.c
> @@ -281,14 +281,6 @@ static struct sk_buff *netem_dequeue(struct Qdisc *sch)
>  			if (unlikely(!skb))
>  				return NULL;
>  
> -#ifdef CONFIG_NET_CLS_ACT
> -			/*
> -			 * If it's at ingress let's pretend the delay is
> -			 * from the network (tstamp will be updated).
> -			 */
> -			if (G_TC_FROM(skb->tc_verd) & AT_INGRESS)
> -				skb->tstamp.tv64 = 0;
> -#endif
>  			pr_debug("netem_dequeue: return skb=%p\n", skb);
>  			sch->q.qlen--;
>  			return skb;
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html