Re: [PATCH net-next 2/2] openvswitch: Add timeout support to ct action

[Yi-Hung Wei <yihung.wei@gmail.com>]

> > +static void ovs_ct_add_timeout(struct net *net, struct nf_conn *ct,
> > +                              const char *timeout_name, u16 l3num, u8 l4num)
> > +{
> This code looks very similar to xt_ct_set_timeout(), can you refactor
> it to avoid code duplication?

Thanks Prvain's feedback. I will export the set timeout function and
use that to avoid duplication.

> >  static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
> > @@ -1550,6 +1607,15 @@ static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
> >                         info->have_eventmask = true;
> >                         info->eventmask = nla_get_u32(a);
> >                         break;
> > +#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
> > +               case OVS_CT_ATTR_TIMEOUT:
> > +                       memcpy(info->timeout, nla_data(a), nla_len(a));
> Before copying timeout, we need to check sizeof source string.
> 'nla_len(a)' needs to be less than CTNL_TIMEOUT_NAME_MAX.

There are some exiting checking in parse_ct() as below. That should
check the sizeof source string already.
....
        nla_for_each_nested(a, attr, rem) {
                int type = nla_type(a);
                int maxlen;
                int minlen;
......
                maxlen = ovs_ct_attr_lens[type].maxlen;
                minlen = ovs_ct_attr_lens[type].minlen;
                if (nla_len(a) < minlen || nla_len(a) > maxlen) {
                        OVS_NLERR(log,
                                  "Conntrack attr type has unexpected
length (type=%d, length=%d, expected=%d)",
                                  type, nla_len(a), maxlen);
                        return -EINVAL;
                }

I will respin v2 soon.

Thanks,

-Yi-Hung