[ovs-dev] [PATCH 2/8] nsh: userland support for network service headers

Pritesh Kothari (pritkoth) pritkoth at cisco.com
Wed Oct 2 01:36:08 UTC 2013


>> 
>> +    {
>> +        MFF_NSP, "nsp", NULL,
>> +        sizeof(ovs_be32), 24,
>> +        MFM_FULLY,
>> +        MFS_HEXADECIMAL,
>> +        MFP_NONE,
>> +        false,
>> +        0, NULL,
>> +        0, NULL,
>> +        OFPUTIL_P_OF10_NXM_ANY,
>> +        OFPUTIL_P_OF10_NXM_ANY,
> 
> These should be:
> 
>        OFPUTIL_P_NONE,
>        OFPUTIL_P_NONE,
> 
> 
> As matching of nsp is not yet enabled.

yep will change.

>> +        { false, false, false, IP_SRC_ANY },   /* remote_ip, in_key. */
>> +        { true,  false, false, IP_SRC_EXACT }, /* remote_ip, local_ip. */
>> +        { true,  false, false, IP_SRC_ANY },   /* remote_ip. */
>> +        { true,  true,  false, IP_SRC_ANY },   /* Flow-based remote. */
>> +        { true,  true,  false, IP_SRC_FLOW },  /* Flow-based everything. */
>> +        { false, false, true,  IP_SRC_EXACT }, /* remote_ip, local_ip, in_key. */
>> +        { false, false, true,  IP_SRC_ANY },   /* remote_ip, in_key. */
>> +        { true,  false, true,  IP_SRC_EXACT }, /* remote_ip, local_ip. */
>> +        { true,  false, true,  IP_SRC_ANY },   /* remote_ip. */
>> +        { true,  true,  true,  IP_SRC_ANY },   /* Flow-based remote. */
>> +        { true,  true,  true,  IP_SRC_FLOW },  /* Flow-based everything. */
>>    };
>> 
> 
> This list gets quite long, and progressively more so with the later patches.
> The list is scanned linearly, hashing and matching on each iteration. Are you
> sure all the combinations you have added are useful?

actually i had included all the cases here, i have shortlisted few, will include those instead.

> Do you envision a
> separate tunnel being set up for each incoming/outgoing NSP pair, or would
> the typical usage set the values as part of the flow match ("in flow")?

actually a specific flow with nsp/nsi pair can come on one vxlan tunnel and go out
on another with different pair, so there are complex cases which needs to be handled,
but, yes, definitely not as many, so will reduce them to minimum required.
yep will document them as well.

> 
>>    const struct tnl_match_pattern *p;
>> @@ -443,6 +459,9 @@ tnl_find(const struct flow *flow) OVS_REQ_RDLOCK(rwlock)
>>        match.in_key_flow = p->in_key_flow;
>>        match.in_key = p->in_key_flow ? 0 : flow->tunnel.tun_id;
>> 
>> +        match.in_nsp_flow = p->in_nsp_flow;
>> +        match.in_nsp = p->in_nsp_flow ? 0 : flow->tunnel.nsp;
>> +
>>        match.ip_dst_flow = p->ip_dst_flow;
>>        match.ip_dst = p->ip_dst_flow ? 0 : flow->tunnel.ip_src;
>> 
>> @@ -477,6 +496,12 @@ tnl_match_fmt(const struct tnl_match *match, struct ds *ds)
>>    



More information about the dev mailing list