Re: [PATCH 2/2] ipv4: Change rt->rt_iif encoding.

	Hello,

On Mon, 23 Jul 2012, David Miller wrote:

From: Julian Anastasov <ja@ssi.bg>
Date: Tue, 24 Jul 2012 03:24:38 +0300 (EEST)

quoted

On Mon, 23 Jul 2012, David Miller wrote:

quoted

Hmmm, the problem is that when we decapsulate VLAN devices, we're left
with the parent device's index in skb->skb_iif.

	Not sure if it is a problem with VLANs, can be also
with some virtual devices but may be they use dev_forward_skb()
where skb_iif is zeroed.

dev_forward_skb() gives the packet to netif_rx() which will thus send
the packet down to __netif_receive_skb() which will set the
skb->skb_iif to the new device's ifindex.  It will not stay at zero
:-)

quoted

	I was also worrying about ip_mc_output looping packets with
skb_iif because skb_clone copies the field but may be such loops
happen only for locally originated traffic where skb_iif starts with
0.

Loopback of multicast packets is done in ip_mc_output(), via
the clone that you mention, via dev_loopback_xmit().

dev_loopback_xmit() gives the packet to netif_rx_ni() which again
lands it back at __netif_receive_skb(), which (with my changes)
will adjust the skb->skb_iif to match whatever sits in skb->dev
at the time.

It should be alright.

	Yes, I know that now it is set in all cases, I was
wondering why __netif_receive_skb was prepared to see
non-zero value. Please also check xfrm4_fill_dst, the
possible case where the new skb_iif usage can differ from
rt_iif is for forwarded packets where skb->dst is changed
with new version, for example:

ip_forward -> *xfrm*_policy_check* -> __xfrm_decode_session ->
_decode_session4 memsets flowi4_iif (now to get skb_iif).

ip_forward -> xfrm4_route_forward -> ... xfrm_lookup ->
... -> xfrm4_fill_dst now will use skb_iif ?

	XFRM is still magic for me and I'm not sure if
we need some other device instead of skb_iif. Should we
prefer dst->dev for output routes? For example:

static inline int inet_iif(const struct sk_buff *skb)
{
	struct rtable *rt = skb_rtable(skb);
	int iif = rt->rt_iif;

	if (iif)
		return iif;
	if (rt_is_output_route(rt))
		return rt->dst.dev->ifindex;
	return skb->skb_iif;
}

	but may be it is same if we restore the
rth->rt_iif	= orig_oif ? : dev_out->ifindex;

	code in __mkroute_output? By this way we will have
correct rt_iif at the time of routing call, before the
packet is looped via this device. Then skb_iif will remain
as default only for input routes. By this way we will be
less worried for places like xfrm or when skb->dst is
replaced in forwarding path. IPIP tunnels also replace
skb->dst but this can hurt only code like route4_classify.

Regards

--
Julian Anastasov [off-list ref]