Re: [PATCH net 2/6] net: openvswitch: conntrack: remove 'add_helper' dead code

[Aaron Conole]

Ilya Maximets <i.maximets@xxxxxxx> writes:

> This variable can only become 'true' when the connection is not
> confirmed, but it is only checked when it is confirmed.  So, it can be
> treated as being always false and just removed.
>
> Fixes: 3c1860543fcc ("openvswitch: add nf_ct_is_confirmed check before
> assigning the helper")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Ilya Maximets <i.maximets@xxxxxxx>
> ---
> @@ -821,13 +818,10 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key,
>  		}
>  
>  		/* Call the helper only if:
> -		 * - nf_conntrack_in() was executed above ("!cached") or a
> -		 *   helper was just attached ("add_helper") for a confirmed
> -		 *   connection, or
> +		 * - nf_conntrack_in() was executed above ("!cached"), or
>  		 * - When committing an unconfirmed connection.
>  		 */
> -		if ((nf_ct_is_confirmed(ct) ? !cached || add_helper :
> -					      info->commit)) {
> +		if ((nf_ct_is_confirmed(ct) ? !cached : info->commit)) {

This is already a good reason to apply - the existing construct wasn't
particularly friendly.  And the follow up in 3/6 is perfect.

>  			int err = nf_ct_helper(skb, ct, ctinfo, info->family);
>  
>  			err = verdict_to_errno(err);

Reviewed-by: Aaron Conole <aconole@xxxxxxxxxx>