netdev - Re: [PATCHv10 ovs 12/15] datapath: Add support for unique flow identifiers.

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]

Message-ID: <CALnjE+p-1+Nh2yHu_zRxrLPnFKnAcW0hDFL1Dex_8ZNUhJ4oNQ@mail.gmail.com>
Date:	Fri, 21 Nov 2014 10:08:56 -0800
From:	Pravin Shelar <pshelar@...ira.com>
To:	Joe Stringer <joestringer@...ira.com>
Cc:	"dev@...nvswitch.org" <dev@...nvswitch.org>,
	netdev <netdev@...r.kernel.org>
Subject: Re: [PATCHv10 ovs 12/15] datapath: Add support for unique flow identifiers.

On Thu, Nov 20, 2014 at 4:33 PM, Joe Stringer <joestringer@...ira.com> wrote:
> On 19 November 2014 15:34, Pravin Shelar <pshelar@...ira.com> wrote:
>> On Thu, Nov 13, 2014 at 11:17 AM, Joe Stringer <joestringer@...ira.com> wrote:
>>> @@ -684,33 +691,43 @@ static size_t ovs_flow_cmd_msg_size(const struct sw_flow_actions *acts)
>>>
>>>  /* Called with ovs_mutex or RCU read lock. */
>>>  static int ovs_flow_cmd_fill_match(const struct sw_flow *flow,
>>> -                                  struct sk_buff *skb)
>>> +                                  struct sk_buff *skb, u32 ufid_flags)
>>>  {
>>>         struct nlattr *nla;
>>>         int err;
>>>
>>> -       /* Fill flow key. */
>>> -       nla = nla_nest_start(skb, OVS_FLOW_ATTR_KEY);
>>> -       if (!nla)
>>> -               return -EMSGSIZE;
>>> -
>>> -       err = ovs_nla_put_flow(&flow->unmasked_key, &flow->unmasked_key, skb,
>>> -                              false);
>>> -       if (err)
>>> -               return err;
>>> -
>>> -       nla_nest_end(skb, nla);
>>> +       /* Fill flow key. If userspace didn't specify a UFID, then ignore the
>>> +        * OMIT_KEY flag. */
>>> +       if (!(ufid_flags & OVS_UFID_F_OMIT_KEY) ||
>>> +           !flow->index_by_ufid) {
>>
>> I am not sure about this check, userspace needs to send atleast ufid
>> or the unmasked key as id for flow. otherwise we shld flag error. Here
>> we can serialize flow->key.
>> There could be another function which takes care of flow-id
>> serialization where we serialize use ufid or unmasked key as flow id.
>> Lets group ufid and unmasked key together rather than masked key and
>> unmasked key which are not related.
>
> Let me start from scratch and see if this is what you're saying.
>
> fill_id() would serialize the UFID or unmasked key if UFID is not available.
> fill_key() would serialize the masked key if !(ufid_flags &
> OVS_UFID_F_OMIT_KEY) and UFID was provided (but not when the UFID is
> missing).
> fill_mask() would serialize the mask if !(ufid_flags & OVS_UFID_F_OMIT_MASK).
>
> I see key and id as related, because the flow_key serves as both the
> "match" and the "id" in the non-ufid case, so we need to know the same
> piece of information in both functions to ensure we don't serialize
> the key twice.

Lets follow new model where ID and key are two separate attributes. I
agree we also need to check for ufid in key serialization function to
handle compatibility code. Keeping masked and unmasked key separate is
easier to understand.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html