netdev - Re: [PATCH nf-next v7 4/7] openvswitch: Find existing conntrack entry after upcall.

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]

Date:	Wed, 17 Feb 2016 15:50:32 -0800
From:	Joe Stringer <joe@....org>
To:	Jarno Rajahalme <jarno@....org>
Cc:	netfilter-devel@...r.kernel.org, netdev <netdev@...r.kernel.org>,
	ovs dev <dev@...nvswitch.org>
Subject: Re: [PATCH nf-next v7 4/7] openvswitch: Find existing conntrack entry
 after upcall.

On 5 February 2016 at 17:41, Jarno Rajahalme <jarno@....org> wrote:
<snip>
>  /* Determine whether skb->nfct is equal to the result of conntrack lookup. */
> -static bool skb_nfct_cached(const struct net *net, const struct sk_buff *skb,
> -                           const struct ovs_conntrack_info *info)
> +static bool skb_nfct_cached(struct net *net,
> +                           const struct sw_flow_key *key,
> +                           const struct ovs_conntrack_info *info,
> +                           struct sk_buff *skb)
>  {
>         enum ip_conntrack_info ctinfo;
>         struct nf_conn *ct;
>
>         ct = nf_ct_get(skb, &ctinfo);
> +       /* If no ct, check if we have evidence that an existing conntrack entry
> +        * might be found for this skb.  This happens when we lose a skb->nfct
> +        * due to an upcall.  If the connection was not confirmed, it is not
> +        * cached and needs to be run through conntrack again. */
> +       if (!ct && key->ct.state & OVS_CS_F_TRACKED
> +           && !(key->ct.state & OVS_CS_F_INVALID)
> +           && key->ct.zone == info->zone.id)
> +               ct = ovs_ct_find_existing(net, &info->zone, info->family, skb,
> +                                         &ctinfo);
>         if (!ct)
>                 return false;

Logically I think that this makes more sense residing within
__ovs_ct_lookup() and actually populating skb->{nfct,nfctinfo} prior
to making this call to skb_nfct_cached() which answers the question
"Is skb->nfct the same as if I did a lookup?". Maybe a better name for
this function is something like ovs_ct_cmp() as it compares the skb's
nfct against the OVS structures.

The call to nf_ct_get() could move out into __ovs_ct_lookup(), then
just pass the 'ct' into here. I see that a later patch already adds
another call to nf_ct_get() into __ovs_ct_lookup(), which should only
be necessary in the case where it is not already cached.