On Feb 11, 2014, at 8:05 AM, Ben Pfaff <b...@nicira.com> wrote:

> On Tue, Feb 11, 2014 at 12:00:46AM +0000, Pritesh Kothari (pritkoth) wrote:
>> Hi Ben,
>> 
>>> -    static const struct tnl_match_pattern patterns[] = {
>>> -        { false, false, IP_SRC_CFG },  /* remote_ip, local_ip, in_key. */
>>> -        { false, false, IP_SRC_ANY },  /* remote_ip, in_key. */
>>> -        { true,  false, IP_SRC_CFG },  /* remote_ip, local_ip. */
>>> -        { true,  false, IP_SRC_ANY },  /* remote_ip. */
>>> -        { true,  true,  IP_SRC_ANY },  /* Flow-based remote. */
>>> -        { true,  true,  IP_SRC_FLOW }, /* Flow-based everything. */
>> 
>> It was lot easier to add specific matches in match pattern here when a tunnel
>> parameter or two gets added. for example when i added nsp and nsi as [1]
>> parameters i could add 8 matches here bringing the total to 12+8 = 20 
>> matches.
> 
> To me, a list of 20 matches sounds unmaintainable.

True.

> 
>> but now here i can?t actually select specific matches there by only
>> giving me option to add (2 * 2 * 3) * 2 * 2 = 48 cases, when i add two
>> new parameters.
> 
> What's wrong with having 48 cases?  I think it's what we should do.

ok good.

> 
>> any insight into this would be greatly appreciated, alternatively i was 
>> thinking
>> of adding the code shown below, outside the outer most for loop in tnl_find
>> to add the 8 matches mentioned above, but then tnl_match_map can?t exactly
>> differentiate these cases from original 12 above, so not sure about it.
>> 
>>    for (in_key_flow = 0; in_key_flow < 2; in_key_flow++) {
>>        for (in_nsp_flow = 0; in_nsp_flow < 2; in_nsp_flow++) {
>>            for (in_nsi_flow = 0; ip_nsi_flow < 2; ip_nsi_flow++) {
> 
> We probably don't want so many nested loops--48 tests is wasteful.  I'd
> suggest instead maintaining a uint64_t with a 1-bit in each position
> where there is any match, and then iterating through the 1-bits with
> bitwise functions.

sounds good to me, will do this and post a patch for this in rfc for nsh soon.

Regards,
Pritesh

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to