On Feb 11, 2014, at 8:05 AM, Ben Pfaff <[email protected]> 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
[email protected]
http://openvswitch.org/mailman/listinfo/dev
