On Wed, Apr 16, 2014 at 11:13 AM, Andy Zhou <[email protected]> wrote:
> On Wed, Apr 16, 2014 at 11:03 AM, Pravin Shelar <[email protected]> wrote:
>> On Wed, Apr 16, 2014 at 6:51 AM, Andy Zhou <[email protected]> wrote:
>>> diff --git a/datapath/actions.c b/datapath/actions.c
>>> index cb239c8..4182b3d 100644
>>> --- a/datapath/actions.c
>>> +++ b/datapath/actions.c
>>> +static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
>>> + const struct nlattr *a)
>>> +{
>>> + struct sw_flow_key recirc_key;
>>> + uint16_t in_port = OVS_CB(skb)->pkt_key->phy.in_port;
>>> + uint32_t dp_hash = OVS_CB(skb)->pkt_key->dp_hash;
>>> + struct vport *p;
>>> + int err;
>>> +
>>> + err = ovs_flow_extract(skb, in_port, &recirc_key);
>>> + if (err)
>>> + return err;
>>> +
>>> + p = ovs_vport_rcu(dp, in_port);
>>> + if (unlikely(!p)) {
>>> + kfree_skb(skb);
>>> + return -ENODEV;
>>> + }
>>> +
>> We can store in-port in ovs-cb to avoid hash lookup.
> Do you mean vport * ? Sure if ovs-cb can afford to lose 8 bytes.
Space isn't really an issue at this point, so it should be fine.
>>> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
>>> index 0a1effa..883d9bf 100644
>>> --- a/datapath/flow_netlink.c
>>> +++ b/datapath/flow_netlink.c
>>> @@ -474,6 +476,23 @@ static int metadata_from_nlattrs(struct sw_flow_match
>>> *match, u64 *attrs,
>>> *attrs &= ~(1ULL << OVS_KEY_ATTR_DP_HASH);
>>> }
>>>
>>> + if (*attrs & (1ULL << OVS_KEY_ATTR_RECIRC_ID)) {
>>> + u32 recirc_id = nla_get_u32(a[OVS_KEY_ATTR_RECIRC_ID]);
>>> +
>>> + if (is_mask && (recirc_id > 0 && recirc_id < UINT_MAX)) {
>>> + OVS_NLERR("Reicrc_id mask is neither wildcard, nor
>>> exact match \n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + SW_FLOW_KEY_PUT(match, recirc_id, recirc_id, is_mask);
>>> + *attrs &= ~(1ULL << OVS_KEY_ATTR_RECIRC_ID);
>>> + }
>>> +
>>> + if (is_mask) {
>>> + /* Always exact match recirc_id. */
>>> + SW_FLOW_KEY_PUT(match, recirc_id, UINT_MAX, is_mask);
>>> + }
>>> +
>>
>> If the recirc_id is zero it should be safe to ignore it. Can you
>> explain need for exact match for recirc_id ?
>
> Consider the following two flows:
>
> 1) in_port = 1 ; actions= A
> 2) recirc_id = 2, in_port =1 : Actions=B
>
> Without forcing exact match of recirc_id, a packet matches 2) could
> also match 1)
Presumably anybody that is using recirculation would already make this
exact match already, right? Is there any possible use case for a
partial mask?
You also have a typo in the error message ("Reicrc_id").
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
