Re: [PATCH nf-next v10 7/8] openvswitch: Delay conntrack helper call for new connections.

[Jarno Rajahalme]

Thanks for the reviews, Joe!

Now we have acks for the patches 3-8, but not for 1 and 2 that touch netfilter 
proper. Who could review those?

  Jarno

> On Mar 10, 2016, at 2:01 PM, Joe Stringer  wrote:
> 
> On 11 March 2016 at 07:54, Jarno Rajahalme  wrote:
>> There is no need to help connections that are not confirmed, so we can
>> delay helping new connections to the time when they are confirmed.
>> This change is needed for NAT support, and having this as a separate
>> patch will make the following NAT patch a bit easier to review.
>> 
>> Signed-off-by: Jarno Rajahalme 
> 
> Acked-by: Joe Stringer 

Re: [PATCH nf-next v10 7/8] openvswitch: Delay conntrack helper call for new connections.

[Joe Stringer]

On 11 March 2016 at 07:54, Jarno Rajahalme  wrote:
> There is no need to help connections that are not confirmed, so we can
> delay helping new connections to the time when they are confirmed.
> This change is needed for NAT support, and having this as a separate
> patch will make the following NAT patch a bit easier to review.
>
> Signed-off-by: Jarno Rajahalme 

Acked-by: Joe Stringer 

[PATCH nf-next v10 7/8] openvswitch: Delay conntrack helper call for new connections.

[Jarno Rajahalme]

There is no need to help connections that are not confirmed, so we can
delay helping new connections to the time when they are confirmed.
This change is needed for NAT support, and having this as a separate
patch will make the following NAT patch a bit easier to review.

Signed-off-by: Jarno Rajahalme 
---
 net/openvswitch/conntrack.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 85256b3..f718b72 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -483,7 +483,11 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
 * actually run the packet through conntrack twice unless it's for a
 * different zone.
 */
-   if (!skb_nfct_cached(net, key, info, skb)) {
+   bool cached = skb_nfct_cached(net, key, info, skb);
+   enum ip_conntrack_info ctinfo;
+   struct nf_conn *ct;
+
+   if (!cached) {
struct nf_conn *tmpl = info->ct;
int err;
 
@@ -506,11 +510,18 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
return -ENOENT;
 
ovs_ct_update_key(skb, info, key, true);
+   }
 
-   if (ovs_ct_helper(skb, info->family) != NF_ACCEPT) {
-   WARN_ONCE(1, "helper rejected packet");
-   return -EINVAL;
-   }
+   /* Call the helper only if:
+* - nf_conntrack_in() was executed above ("!cached") for a confirmed
+*   connection, or
+* - When committing an unconfirmed connection.
+*/
+   ct = nf_ct_get(skb, &ctinfo);
+   if (ct && (nf_ct_is_confirmed(ct) ? !cached : info->commit) &&
+   ovs_ct_helper(skb, info->family) != NF_ACCEPT) {
+   WARN_ONCE(1, "helper rejected packet");
+   return -EINVAL;
}
 
return 0;
-- 
2.1.4