| Message ID | 1461187870-54322-1-git-send-email-jarno@ovn.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
On Wed, Apr 20, 2016 at 02:31:10PM -0700, Jarno Rajahalme wrote: > Clear the skb hash when it does not reflect the actual header values > any more. > > Signed-off-by: Jarno Rajahalme <jarno@ovn.org> > --- > net/netfilter/nf_nat_core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c > index 06a9f45..3c2302f 100644 > --- a/net/netfilter/nf_nat_core.c > +++ b/net/netfilter/nf_nat_core.c > @@ -505,6 +505,7 @@ unsigned int nf_nat_packet(struct nf_conn *ct, > if (!l3proto->manip_pkt(skb, 0, l4proto, &target, mtype)) > return NF_DROP; > } > + skb_clear_hash(skb); > return NF_ACCEPT; > } Cc'ing Florian. This seems to affect the new tracing infrastructure for nf_tables: 31 static int trace_fill_id(struct sk_buff *nlskb, struct sk_buff *skb) 32 { 33 __be32 id; 34 35 /* using skb address as ID results in a limited number of 36 * values (and quick reuse). 37 * 38 * So we attempt to use as many skb members that will not 39 * change while skb is with netfilter. 40 */ 41 id = (__be32)jhash_2words(hash32_ptr(skb), skb_get_hash(skb), 42 skb->skb_iif); 43 44 return nla_put_be32(nlskb, NFTA_TRACE_ID, id); 45 } -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Wed, Apr 20, 2016 at 02:31:10PM -0700, Jarno Rajahalme wrote: > > Clear the skb hash when it does not reflect the actual header values > > any more. > > > > Signed-off-by: Jarno Rajahalme <jarno@ovn.org> > > --- > > net/netfilter/nf_nat_core.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c > > index 06a9f45..3c2302f 100644 > > --- a/net/netfilter/nf_nat_core.c > > +++ b/net/netfilter/nf_nat_core.c > > @@ -505,6 +505,7 @@ unsigned int nf_nat_packet(struct nf_conn *ct, > > if (!l3proto->manip_pkt(skb, 0, l4proto, &target, mtype)) > > return NF_DROP; > > } > > + skb_clear_hash(skb); > > return NF_ACCEPT; > > } > > Cc'ing Florian. > > This seems to affect the new tracing infrastructure for nf_tables: Right. Jarno, what is your patch trying to fix? -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jarno Rajahalme <jarno@ovn.org> wrote: > In the OVS datapath we clear the skb hash whenever we change any of the fields that may be used to compute it. This guarantees that any given flow will get the same hash value when assigning packets to bond interfaces based on the skb hash. If the hash is not cleared, some packets may use the pre-nat hash provided by a nic, for example, while others may not have the nic provided hash and compute a new one post-nat. Also, if DNAT is used for load balancing, the hash should be computed after the NAT for the difference in the destination address to make a difference in the hash value. > > We could also clear the hash in the OVS datapath code after calling into nf nat, but could that still have the same issue with affecting the nf_tables tracing (of which I know nothing about)? No, that would not affect nft. Note that we could also remove the skb_get_hash() usage in nf trace (we want to compute a pseudo-id that doesn't change while skb travels the netfilter hooks). For now I'd prefer if the hash reset would happen in OVS. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" 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/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c index 06a9f45..3c2302f 100644 --- a/net/netfilter/nf_nat_core.c +++ b/net/netfilter/nf_nat_core.c @@ -505,6 +505,7 @@ unsigned int nf_nat_packet(struct nf_conn *ct, if (!l3proto->manip_pkt(skb, 0, l4proto, &target, mtype)) return NF_DROP; } + skb_clear_hash(skb); return NF_ACCEPT; } EXPORT_SYMBOL_GPL(nf_nat_packet);
Clear the skb hash when it does not reflect the actual header values any more. Signed-off-by: Jarno Rajahalme <jarno@ovn.org> --- net/netfilter/nf_nat_core.c | 1 + 1 file changed, 1 insertion(+)