Thread (10 messages) 10 messages, 2 authors, 2019-06-26

Re: [PATCH net-next 2/5] net: sched: em_ipt: set the family based on the protocol when matching

From: Eyal Birger <hidden>
Date: 2019-06-26 16:23:02

On Wed, 26 Jun 2019 16:45:28 +0300
Nikolay Aleksandrov [off-list ref] wrote:
On 26/06/2019 16:33, Eyal Birger wrote:
quoted
Hi Nikolay,
   
On Wed, 26 Jun 2019 14:58:52 +0300
Nikolay Aleksandrov [off-list ref] wrote:
  
quoted
Set the family based on the protocol otherwise protocol-neutral
matches will have wrong information (e.g. NFPROTO_UNSPEC). In
preparation for using NFPROTO_UNSPEC xt matches.

Signed-off-by: Nikolay Aleksandrov <redacted>
---
 net/sched/em_ipt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
...
quoted
quoted
-	nf_hook_state_init(&state, im->hook, im->match->family,
+	nf_hook_state_init(&state, im->hook, state.pf,
 			   indev ?: skb->dev, skb->dev, NULL,
em->net, NULL); 
 	acpar.match = im->match;  
I think this change is incompatible with current behavior.

Consider the 'policy' match which matches the packet's xfrm state
(sec_path) with the provided user space parameters. The sec_path
includes information about the encapsulating packet's parameters
whereas the current skb points to the encapsulated packet, and the
match is done on the encapsulating packet's info.

So if you have an IPv6 packet encapsulated within an IPv4 packet,
the match parameters should be done using IPv4 parameters, not IPv6.

Maybe use the packet's family only if the match family is UNSPEC?

Eyal.
  
Hi Eyal,
I see your point, I was wondering about the xfrm cases. :)
In such case I think we can simplify the set and do it only on UNSPEC
matches as you suggest.

Maybe we should enforce the tc protocol based on the user-specified
nfproto at least from iproute2 otherwise people can add mismatching
rules (e.g. nfproto == v6, tc proto == v4).
Hi Nik,

I think for iproute2 the issue is the same. For encapsulated IPv6 in
IPv4 for example, tc proto will be IPv6 (tc sees the encapsulated
packet after decryption) whereas nfproto will be IPv4 (policy match is
done on the encapsulating state metadata which is IPv4).

I think the part missing in iproute2 is the ability to specify
NFPROTO_UNSPEC.

Thanks,
Eyal
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help