[ovs-dev] [PATCH] datapath: Use a single attribute array for parsing values and masks.

[Jesse Gross]

Thanks, I applied it.

It was Pravin's compiler, not mine, so I'm not exactly sure but I
believe that it is an older version of GCC. The message is that the
stack size exceeds 1024 bytes (which is not automatically a problem
but given that the kernel has a fixed stack size it can be dangerous
if you have a few large frames).

On Thu, Jun 20, 2013 at 5:19 PM, Andy Zhou <azhou at nicira.com> wrote:
> Looks good.  Just curious, which compiler warn about this? What's the
> warning message?
>
>
> On Thu, Jun 20, 2013 at 5:15 PM, Jesse Gross <jesse at nicira.com> wrote:
>>
>> When parsing flow Netlink messages we currently have arrays to hold the
>> attribute pointers for both values and masks. This results in a large
>> stack, which some compilers warn about. It's not actually necessary
>> to have both arrays at the same time, so we can collapse this to a
>> single array.
>>
>> Reported-by: Pravin B Shelar <pshelar at nicira.com>
>> Signed-off-by: Jesse Gross <jesse at nicira.com>
>> ---
>>  datapath/flow.c | 13 ++++++-------
>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/datapath/flow.c b/datapath/flow.c
>> index 79d5d49..3a7bc9b 100644
>> --- a/datapath/flow.c
>> +++ b/datapath/flow.c
>> @@ -1479,7 +1479,6 @@ int ovs_match_from_nlattrs(struct sw_flow_match
>> *match,
>>                            const struct nlattr *mask)
>>  {
>>         const struct nlattr *a[OVS_KEY_ATTR_MAX + 1];
>> -       const struct nlattr *m[OVS_KEY_ATTR_MAX + 1];
>>         const struct nlattr *encap;
>>         u64 key_attrs = 0;
>>         u64 mask_attrs = 0;
>> @@ -1516,19 +1515,19 @@ int ovs_match_from_nlattrs(struct sw_flow_match
>> *match,
>>                 return err;
>>
>>         if (mask) {
>> -               err = parse_flow_mask_nlattrs(mask, m, &mask_attrs);
>> +               err = parse_flow_mask_nlattrs(mask, a, &mask_attrs);
>>                 if (err)
>>                         return err;
>>
>>                 if ((mask_attrs & 1ULL << OVS_KEY_ATTR_ENCAP) &&
>> encap_valid) {
>>                         __be16 eth_type = 0;
>>
>> -                       if (m[OVS_KEY_ATTR_ETHERTYPE])
>> -                               eth_type =
>> nla_get_be16(m[OVS_KEY_ATTR_ETHERTYPE]);
>> +                       if (a[OVS_KEY_ATTR_ETHERTYPE])
>> +                               eth_type =
>> nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]);
>>                         if (eth_type == htons(0xffff)) {
>>                                 mask_attrs &= ~(1ULL <<
>> OVS_KEY_ATTR_ETHERTYPE);
>> -                               encap = m[OVS_KEY_ATTR_ENCAP];
>> -                               err = parse_flow_mask_nlattrs(encap, m,
>> &mask_attrs);
>> +                               encap = a[OVS_KEY_ATTR_ENCAP];
>> +                               err = parse_flow_mask_nlattrs(encap, a,
>> &mask_attrs);
>>                         } else
>>                                 err = -EINVAL;
>>
>> @@ -1536,7 +1535,7 @@ int ovs_match_from_nlattrs(struct sw_flow_match
>> *match,
>>                                 return err;
>>                 }
>>
>> -               err = ovs_key_from_nlattrs(match,  mask_attrs, m, true);
>> +               err = ovs_key_from_nlattrs(match, mask_attrs, a, true);
>>                 if (err)
>>                         return err;
>>         } else {
>> --
>> 1.8.1.2
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
>