On 11/27/2019 5:55 PM, Marcelo Ricardo Leitner wrote:
> On Wed, Nov 27, 2019 at 02:55:16PM +0200, Roi Dayan wrote:
>> @@ -1367,7 +1387,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match
>> *match,
>> uint32_t block_id = 0;
>> struct nlattr *nla;
>> struct tc_id id;
>> - uint32_t chain;
>> + uint32_t chain = 0;
>> size_t left;
>> int prio = 0;
>> int ifindex;
>> @@ -1382,7 +1402,13 @@ netdev_tc_flow_put(struct netdev *netdev, struct
>> match *match,
>>
>> memset(&flower, 0, sizeof flower);
>>
>> - chain = key->recirc_id;
>> + if (key->recirc_id) {
>> + if (recirc_id_sharing_support(info->dpif)) {
>> + chain = key->recirc_id;
>> + } else {
>> + return EOPNOTSUPP;
>> + }
>> + }
>> mask->recirc_id = 0;
> It may be a personal preference, but I find this easier to read:
>
> chain = key->recirc_id;
> mask->recirc_id = 0;
> + if (unlikely(chain && !recirc_id_sharing_support(info->dpif))) {
> + return EOPNOTSUPP;
> + }
>
> As it avoids the cascading conditions. Just a suggestion..
Will do.
This reminds me I should probably move this patch before the chains
support patch.
Thanks for the review. will post V3 soon.
>
>>
>> if (flow_tnl_dst_is_set(&key->tunnel)) {
>> @@ -1630,7 +1656,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match
>> *match,
>> action = &flower.actions[flower.action_count];
>> if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
>> odp_port_t port = nl_attr_get_odp_port(nla);
>> - struct netdev *outdev = netdev_ports_get(port,
>> info->dpif_class);
>> + struct netdev *outdev = netdev_ports_get(port,
>> +
>> info->dpif->dpif_class);
>>
>> action->out.ifindex_out = netdev_get_ifindex(outdev);
>> action->out.ingress =
>> is_internal_port(netdev_get_type(outdev));
>> @@ -1696,6 +1723,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match
>> *match,
>> action->ct.clear = true;
>> flower.action_count++;
>> } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_RECIRC) {
>> + if (!recirc_id_sharing_support(info->dpif)) {
>> + return EOPNOTSUPP;
>> + }
>> action->type = TC_ACT_GOTO;
>> action->chain = nl_attr_get_u32(nla);
>> flower.action_count++;
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
