diff mbox series

[ovs-dev,PATCHv2] ofproto-dpif: Expose datapath capability to ovsdb.

Message ID 1570228328-33373-1-git-send-email-u9012063@gmail.com
State Superseded
Headers show
Series [ovs-dev,PATCHv2] ofproto-dpif: Expose datapath capability to ovsdb. | expand

Commit Message

William Tu Oct. 4, 2019, 10:32 p.m. UTC
The patch adds support for fetching the datapath's capabilities
from the result of 'check_support()', and write the supported capability
to a new database column, called 'capabilities' under Datapath table.

To see how it works, run:
 # ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
 # ovs-vsctl -- --id=@m create Datapath datapath_version=0 \
     'ct_zones={}' 'capabilities={}' \
     -- set Open_vSwitch . datapaths:"netdev"=@m

 # ovs-vsctl list-dp-cap netdev
 ufid=true sample_nesting=true clone=true tnl_push_pop=true \
 ct_orig_tuple=true ct_eventmask=true ct_state=true \
 ct_clear=true max_vlan_headers=1 recirc=true ct_label=true \
 max_hash_alg=1 ct_state_nat=true ct_timeout=true \
 ct_mark=true ct_orig_tuple6=true check_pkt_len=true \
 masked_set_action=true max_mpls_depth=3 trunc=true ct_zone=true

Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/593749381
Signed-off-by: William Tu <u9012063@gmail.com>
---
v2:
	rebase to master
---
 ofproto/ofproto-dpif.c     | 51 +++++++++++++++++++++++++++++++
 ofproto/ofproto-provider.h |  2 ++
 ofproto/ofproto.c          | 12 ++++++++
 ofproto/ofproto.h          |  2 ++
 tests/ovs-vsctl.at         | 10 ++++++
 utilities/ovs-vsctl.8.in   |  6 ++++
 utilities/ovs-vsctl.c      | 28 +++++++++++++++++
 vswitchd/bridge.c          | 21 +++++++++++++
 vswitchd/vswitch.ovsschema |  5 ++-
 vswitchd/vswitch.xml       | 76 ++++++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 212 insertions(+), 1 deletion(-)

Comments

Gregory Rose Oct. 15, 2019, 9:06 p.m. UTC | #1
On 10/4/2019 3:32 PM, William Tu wrote:
> The patch adds support for fetching the datapath's capabilities
> from the result of 'check_support()', and write the supported capability
> to a new database column, called 'capabilities' under Datapath table.
>
> To see how it works, run:
>   # ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
>   # ovs-vsctl -- --id=@m create Datapath datapath_version=0 \
>       'ct_zones={}' 'capabilities={}' \
>       -- set Open_vSwitch . datapaths:"netdev"=@m
>
>   # ovs-vsctl list-dp-cap netdev
>   ufid=true sample_nesting=true clone=true tnl_push_pop=true \
>   ct_orig_tuple=true ct_eventmask=true ct_state=true \
>   ct_clear=true max_vlan_headers=1 recirc=true ct_label=true \
>   max_hash_alg=1 ct_state_nat=true ct_timeout=true \
>   ct_mark=true ct_orig_tuple6=true check_pkt_len=true \
>   masked_set_action=true max_mpls_depth=3 trunc=true ct_zone=true
>
> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/593749381
> Signed-off-by: William Tu <u9012063@gmail.com>

Patch works as advertised and the code looks fine to me.

Tested-by: Greg Rose <gvrose8192@gmail.com>
Reviewed-by: Greg Rose <gvrose8192@gmail.com>

Thanks William!

- Greg

> ---
> v2:
> 	rebase to master
> ---
>   ofproto/ofproto-dpif.c     | 51 +++++++++++++++++++++++++++++++
>   ofproto/ofproto-provider.h |  2 ++
>   ofproto/ofproto.c          | 12 ++++++++
>   ofproto/ofproto.h          |  2 ++
>   tests/ovs-vsctl.at         | 10 ++++++
>   utilities/ovs-vsctl.8.in   |  6 ++++
>   utilities/ovs-vsctl.c      | 28 +++++++++++++++++
>   vswitchd/bridge.c          | 21 +++++++++++++
>   vswitchd/vswitch.ovsschema |  5 ++-
>   vswitchd/vswitch.xml       | 76 ++++++++++++++++++++++++++++++++++++++++++++++
>   10 files changed, 212 insertions(+), 1 deletion(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 496a16c8a4af..c37181664523 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -5440,6 +5440,56 @@ ct_del_zone_timeout_policy(const char *datapath_type, uint16_t zone_id)
>       }
>   }
>   
> +static void
> +get_datapath_cap(const char *datapath_type, struct smap *cap)
> +{
> +    char *str_value;
> +    struct odp_support odp;
> +    struct dpif_backer_support s;
> +    struct dpif_backer *backer = shash_find_data(&all_dpif_backers,
> +                                                 datapath_type);
> +    if (!backer) {
> +        return;
> +    }
> +    s = backer->rt_support;
> +    odp = s.odp;
> +
> +    /* ODP_SUPPORT_FIELDS */
> +    str_value = xasprintf("%lu", odp.max_vlan_headers);
> +    smap_add(cap, "max_vlan_headers", str_value);
> +    free(str_value);
> +
> +    str_value = xasprintf("%lu", odp.max_mpls_depth);
> +    smap_add(cap, "max_mpls_depth", str_value);
> +    free(str_value);
> +
> +    smap_add(cap, "recirc", odp.recirc ? "true" : "false");
> +    smap_add(cap, "ct_state", odp.ct_state ? "true" : "false");
> +    smap_add(cap, "ct_zone", odp.ct_zone ? "true" : "false");
> +    smap_add(cap, "ct_mark", odp.ct_mark ? "true" : "false");
> +    smap_add(cap, "ct_label", odp.ct_label ? "true" : "false");
> +    smap_add(cap, "ct_state_nat", odp.ct_state_nat ? "true" : "false");
> +    smap_add(cap, "ct_orig_tuple", odp.ct_orig_tuple ? "true" : "false");
> +    smap_add(cap, "ct_orig_tuple6", odp.ct_orig_tuple6 ? "true" : "false");
> +
> +    /* DPIF_SUPPORT_FIELDS */
> +    smap_add(cap, "masked_set_action", s.masked_set_action ? "true" : "false");
> +    smap_add(cap, "tnl_push_pop", s.tnl_push_pop ? "true" : "false");
> +    smap_add(cap, "ufid", s.ufid ? "true" : "false");
> +    smap_add(cap, "trunc", s.trunc ? "true" : "false");
> +    smap_add(cap, "clone", s.clone ? "true" : "false");
> +    smap_add(cap, "sample_nesting", s.sample_nesting ? "true" : "false");
> +    smap_add(cap, "ct_eventmask", s.ct_eventmask ? "true" : "false");
> +    smap_add(cap, "ct_clear", s.ct_clear ? "true" : "false");
> +
> +    str_value = xasprintf("%lu", s.max_hash_alg);
> +    smap_add(cap, "max_hash_alg", str_value);
> +    free(str_value);
> +
> +    smap_add(cap, "check_pkt_len", s.check_pkt_len ? "true" : "false");
> +    smap_add(cap, "ct_timeout", s.ct_timeout ? "true" : "false");
> +}
> +
>   /* Gets timeout policy name in 'backer' based on 'zone', 'dl_type' and
>    * 'nw_proto'.  Returns true if the zone-based timeout policy is configured.
>    * On success, stores the timeout policy name in 'tp_name', and sets
> @@ -6581,4 +6631,5 @@ const struct ofproto_class ofproto_dpif_class = {
>       ct_flush,                   /* ct_flush */
>       ct_set_zone_timeout_policy,
>       ct_del_zone_timeout_policy,
> +    get_datapath_cap,
>   };
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index c980e6bffff5..80c4fee06d01 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1889,6 +1889,8 @@ struct ofproto_class {
>       /* Deletes the timeout policy associated with 'zone' in datapath type
>        * 'dp_type'. */
>       void (*ct_del_zone_timeout_policy)(const char *dp_type, uint16_t zone);
> +    /* Get the datapath's capabilities. */
> +    void (*get_datapath_cap)(const char *dp_type, struct smap *caps);
>   };
>   
>   extern const struct ofproto_class ofproto_dpif_class;
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index b4249b0d8818..b3d68e74af3f 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -954,6 +954,18 @@ ofproto_get_flow_restore_wait(void)
>       return flow_restore_wait;
>   }
>   
> +/* Retrieve datapath capabilities. */
> +void
> +ofproto_get_datapath_cap(const char *datapath_type, struct smap *dp_cap)
> +{
> +    datapath_type = ofproto_normalize_type(datapath_type);
> +    const struct ofproto_class *class = ofproto_class_find__(datapath_type);
> +
> +    if (class->get_datapath_cap) {
> +        class->get_datapath_cap(datapath_type, dp_cap);
> +    }
> +}
> +
>   /* Connection tracking configuration. */
>   void
>   ofproto_ct_set_zone_timeout_policy(const char *datapath_type, uint16_t zone_id,
> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
> index 033c4cf93e94..48a9d602c214 100644
> --- a/ofproto/ofproto.h
> +++ b/ofproto/ofproto.h
> @@ -372,6 +372,8 @@ void ofproto_ct_set_zone_timeout_policy(const char *datapath_type,
>                                           struct simap *timeout_policy);
>   void ofproto_ct_del_zone_timeout_policy(const char *datapath_type,
>                                           uint16_t zone);
> +void ofproto_get_datapath_cap(const char *datapath_type,
> +                              struct smap *dp_cap);
>   
>   /* Configuration of ports. */
>   void ofproto_port_unregister(struct ofproto *, ofp_port_t ofp_port);
> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> index 4d55f89e4ae3..18b9cd7c89a5 100644
> --- a/tests/ovs-vsctl.at
> +++ b/tests/ovs-vsctl.at
> @@ -819,6 +819,10 @@ AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
>   AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=1])])
>   AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
>   ])
> +
> +AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 'capabilities={recirc=true}' -- set Open_vSwitch . datapaths:"system"=@m])], [0], [stdout])
> +AT_CHECK([RUN_OVS_VSCTL([list-dp-cap system])], [0], [recirc=true
> +])
>   OVS_VSCTL_CLEANUP
>   AT_CLEANUP
>   
> @@ -962,6 +966,12 @@ AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=11])],
>   ])
>   AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
>   ])
> +
> +AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 'capabilities={recirc=true}' -- set Open_vSwitch . datapaths:"system"=@m])], [0], [stdout])
> +AT_CHECK([RUN_OVS_VSCTL([list-dp-cap nosystem])],
> +  [1], [], [ovs-vsctl: datapath "nosystem" record not found
> +])
> +
>   OVS_VSCTL_CLEANUP
>   AT_CLEANUP
>   
> diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
> index 14a8aa4a48ac..ff97922dd0c6 100644
> --- a/utilities/ovs-vsctl.8.in
> +++ b/utilities/ovs-vsctl.8.in
> @@ -379,6 +379,12 @@ delete a zone that does not exist has no effect.
>   .IP "\fBlist\-zone\-tp \fIdatapath\fR"
>   Prints the timeout policies of all zones in \fIdatapath\fR.
>   .
> +.SS "Datapath Capabilities Command"
> +The command query datapath capabilities.
> +.
> +.IP "\fBlist\-dp\-cap \fIdatapath\fR"
> +Prints the datapath's capabilities.
> +.
>   .SS "OpenFlow Controller Connectivity"
>   .
>   \fBovs\-vswitchd\fR can perform all configured bridging and switching
> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> index 7232471e68b9..bd3972636e66 100644
> --- a/utilities/ovs-vsctl.c
> +++ b/utilities/ovs-vsctl.c
> @@ -1363,6 +1363,31 @@ pre_get_zone(struct ctl_context *ctx)
>   }
>   
>   static void
> +pre_get_dp_cap(struct ctl_context *ctx)
> +{
> +    ovsdb_idl_add_column(ctx->idl, &ovsrec_open_vswitch_col_datapaths);
> +    ovsdb_idl_add_column(ctx->idl, &ovsrec_datapath_col_capabilities);
> +}
> +
> +static void
> +cmd_list_dp_cap(struct ctl_context *ctx)
> +{
> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
> +    struct smap_node *node;
> +
> +    struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, ctx->argv[1]);
> +    if (!dp) {
> +        ctl_fatal("datapath \"%s\" record not found", ctx->argv[1]);
> +    }
> +
> +    SMAP_FOR_EACH (node, &dp->capabilities) {
> +        ds_put_format(&ctx->output, "%s=%s ",node->key, node->value);
> +    }
> +    ds_chomp(&ctx->output, ' ');
> +    ds_put_char(&ctx->output, '\n');
> +}
> +
> +static void
>   cmd_add_br(struct ctl_context *ctx)
>   {
>       struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
> @@ -3112,6 +3137,9 @@ static const struct ctl_command_syntax vsctl_commands[] = {
>        "--if-exists", RW},
>       {"list-zone-tp", 1, 1, "", pre_get_zone, cmd_list_zone_tp, NULL, "", RO},
>   
> +    /* Datapath capabilities. */
> +    {"list-dp-cap", 1, 1, "", pre_get_dp_cap, cmd_list_dp_cap, NULL, "", RO},
> +
>       {NULL, 0, 0, NULL, NULL, NULL, NULL, NULL, RO},
>   };
>   
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 9095ebf5d5e9..d402e39bdc68 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -171,6 +171,7 @@ struct datapath {
>       struct hmap ct_zones;       /* Map of 'struct ct_zone' elements, indexed
>                                    * by 'zone'. */
>       struct hmap_node node;      /* Node in 'all_datapaths' hmap. */
> +    struct smap caps;           /* Capabilities. */
>       unsigned int last_used;     /* The last idl_seqno that this 'datapath'
>                                    * used in OVSDB. This number is used for
>                                    * garbage collection. */
> @@ -700,6 +701,7 @@ datapath_create(const char *type)
>       dp->type = xstrdup(type);
>       hmap_init(&dp->ct_zones);
>       hmap_insert(&all_datapaths, &dp->node, hash_string(type, 0));
> +    smap_init(&dp->caps);
>       return dp;
>   }
>   
> @@ -716,6 +718,7 @@ datapath_destroy(struct datapath *dp)
>           hmap_remove(&all_datapaths, &dp->node);
>           hmap_destroy(&dp->ct_zones);
>           free(dp->type);
> +        smap_destroy(&dp->caps);
>           free(dp);
>       }
>   }
> @@ -759,6 +762,23 @@ ct_zones_reconfigure(struct datapath *dp, struct ovsrec_datapath *dp_cfg)
>   }
>   
>   static void
> +dp_capability_reconfigure(struct datapath *dp,
> +                          struct ovsrec_datapath *dp_cfg)
> +{
> +    struct smap_node *node;
> +    struct smap cap;
> +
> +    smap_init(&cap);
> +    ofproto_get_datapath_cap(dp->type, &cap);
> +
> +    SMAP_FOR_EACH (node, &cap) {
> +        ovsrec_datapath_update_capabilities_setkey(dp_cfg, node->key,
> +                                                   node->value);
> +    }
> +    smap_destroy(&cap);
> +}
> +
> +static void
>   datapath_reconfigure(const struct ovsrec_open_vswitch *cfg)
>   {
>       struct datapath *dp, *next;
> @@ -771,6 +791,7 @@ datapath_reconfigure(const struct ovsrec_open_vswitch *cfg)
>           dp = datapath_lookup(dp_name);
>           if (!dp) {
>               dp = datapath_create(dp_name);
> +            dp_capability_reconfigure(dp, dp_cfg);
>           }
>           dp->last_used = idl_seqno;
>           ct_zones_reconfigure(dp, dp_cfg);
> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> index 02be5ddeec97..0666c8c76446 100644
> --- a/vswitchd/vswitch.ovsschema
> +++ b/vswitchd/vswitch.ovsschema
> @@ -1,6 +1,6 @@
>   {"name": "Open_vSwitch",
>    "version": "8.2.0",
> - "cksum": "4076590391 26298",
> + "cksum": "1076640191 26427",
>    "tables": {
>      "Open_vSwitch": {
>        "columns": {
> @@ -650,6 +650,9 @@
>                     "value": {"type": "uuid",
>                               "refTable": "CT_Zone"},
>                     "min": 0, "max": "unlimited"}},
> +       "capabilities": {
> +         "type": {"key": "string", "value": "string",
> +                  "min": 0, "max": "unlimited"}},
>          "external_ids": {
>            "type": {"key": "string", "value": "string",
>                     "min": 0, "max": "unlimited"}}}},
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 01304a5ed2c2..6201f5fd7886 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -5650,6 +5650,82 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
>         connection tracking-related OpenFlow matches and actions).
>       </column>
>   
> +    <column name="capabilities" key="max_vlan_headers"
> +            type='{"type": "integer", "minInteger": 0}'>
> +      Maximum number of 802.1q VLAN headers to serialize in a mask.
> +    </column>
> +    <column name="capabilities" key="max_mpls_depth"
> +            type='{"type": "integer", "minInteger": 0}'>
> +      Maximum number of MPLS label stack entries to serialise in a mask.
> +    </column>
> +    <column name="capabilities" key="recirc" type='{"type": "boolean"}'>
> +      If this is true, then recirculation fields will always be serialised.
> +    </column>
> +    <column name="capabilities" key="ct_state" type='{"type": "boolean"}'>
> +      If true, datapath supports OVS_KEY_ATTR_CT_STATE.
> +    </column>
> +    <column name="capabilities" key="ct_zone" type='{"type": "boolean"}'>
> +      If true, datapath supports OVS_KEY_ATTR_CT_ZONE.
> +    </column>
> +    <column name="capabilities" key="ct_mark" type='{"type": "boolean"}'>
> +      If true, datapath supports OVS_KEY_ATTR_CT_MARK.
> +    </column>
> +    <column name="capabilities" key="ct_label" type='{"type": "boolean"}'>
> +      If true, datapath supports OVS_KEY_ATTR_CT_LABEL.
> +    </column>
> +    <column name="capabilities" key="ct_state_nat" type='{"type": "boolean"}'>
> +      If true, it means that the datapath supports the NAT bits in
> +      'ct_state'.  The above 'ct_state' member must be true for this
> +      to make sense.
> +    </column>
> +    <column name="capabilities" key="ct_orig_tuple"
> +            type='{"type": "boolean"}'>
> +      Conntrack original direction tuple matching * supported.
> +    </column>
> +    <column name="capabilities" key="ct_orig_tuple6"
> +            type='{"type": "boolean"}'>
> +      Conntrack original direction tuple6 matching * supported.
> +    </column>
> +    <column name="capabilities" key="masked_set_action"
> +            type='{"type": "boolean"}'>
> +      True if the datapath supports masked data in
> +      OVS_ACTION_ATTR_SET actions.
> +    </column>
> +    <column name="capabilities" key="tnl_push_pop" type='{"type": "boolean"}'>
> +      True if the datapath supports tnl_push and pop actions.
> +    </column>
> +    <column name="capabilities" key="ufid" type='{"type": "boolean"}'>
> +      True if the datapath supports OVS_FLOW_ATTR_UFID.
> +    </column>
> +    <column name="capabilities" key="trunc" type='{"type": "boolean"}'>
> +      True if the datapath supports OVS_ACTION_ATTR_TRUNC action.
> +    </column>
> +    <column name="capabilities" key="clone" type='{"type": "boolean"}'>
> +      True if the datapath supports OVS_ACTION_ATTR_CLONE action.
> +    </column>
> +    <column name="capabilities" key="sample_nesting"
> +            type='{"type": "integer", "minInteger": 0}'>
> +      Maximum level of nesting allowed by OVS_ACTION_ATTR_SAMPLE action.
> +    </column>
> +    <column name="capabilities" key="ct_eventmask" type='{"type": "boolean"}'>
> +      OVS_CT_ATTR_EVENTMASK supported by OVS_ACTION_ATTR_CT action.
> +    </column>
> +    <column name="capabilities" key="ct_clear" type='{"type": "boolean"}'>
> +      True if the datapath supports OVS_ACTION_ATTR_CT_CLEAR action.
> +    </column>
> +    <column name="capabilities" key="max_hash_alg"
> +            type='{"type": "integer", "minInteger": 0}'>
> +      Highest supported dp_hash algorithm.
> +    </column>
> +    <column name="capabilities" key="check_pkt_len"
> +            type='{"type": "boolean"}'>
> +      True if the datapath supports OVS_ACTION_ATTR_CHECK_PKT_LEN.
> +    </column>
> +    <column name="capabilities" key="ct_timeout" type='{"type": "boolean"}'>
> +      True if the datapath supports OVS_CT_ATTR_TIMEOUT in
> +      OVS_ACTION_ATTR_CTaction.
> +    </column>
> +
>       <group title="Common Columns">
>         The overall purpose of these columns is described under <code>Common
>         Columns</code> at the beginning of this document.
Gregory Rose Oct. 16, 2019, 4:35 p.m. UTC | #2
On 10/15/2019 2:06 PM, Gregory Rose wrote:
>
> On 10/4/2019 3:32 PM, William Tu wrote:
>> The patch adds support for fetching the datapath's capabilities
>> from the result of 'check_support()', and write the supported capability
>> to a new database column, called 'capabilities' under Datapath table.
>>
>> To see how it works, run:
>>   # ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
>>   # ovs-vsctl -- --id=@m create Datapath datapath_version=0 \
>>       'ct_zones={}' 'capabilities={}' \
>>       -- set Open_vSwitch . datapaths:"netdev"=@m
>>
>>   # ovs-vsctl list-dp-cap netdev
>>   ufid=true sample_nesting=true clone=true tnl_push_pop=true \
>>   ct_orig_tuple=true ct_eventmask=true ct_state=true \
>>   ct_clear=true max_vlan_headers=1 recirc=true ct_label=true \
>>   max_hash_alg=1 ct_state_nat=true ct_timeout=true \
>>   ct_mark=true ct_orig_tuple6=true check_pkt_len=true \
>>   masked_set_action=true max_mpls_depth=3 trunc=true ct_zone=true
>>
>> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/593749381
>> Signed-off-by: William Tu <u9012063@gmail.com>
>
> Patch works as advertised and the code looks fine to me.
>
> Tested-by: Greg Rose <gvrose8192@gmail.com>
> Reviewed-by: Greg Rose <gvrose8192@gmail.com>

I have to take this back.  While doing other work I found that this 
patch has an issue on 32bit
architecture:

ofproto/ofproto-dpif.c:In function ‘get_datapath_cap’:

ofproto/ofproto-dpif.c:5458:27:error: format ‘%lu’ expects argument of 
type ‘long unsigned int’, but argument 2 has type ‘size_t {aka unsigned 
int}’ [-Werror=format=]

str_value = xasprintf("%lu", odp.max_vlan_headers);

^

ofproto/ofproto-dpif.c:5462:27:error: format ‘%lu’ expects argument of 
type ‘long unsigned int’, but argument 2 has type ‘size_t {aka unsigned 
int}’ [-Werror=format=]

str_value = xasprintf("%lu", odp.max_mpls_depth);

^

ofproto/ofproto-dpif.c:5485:27:error: format ‘%lu’ expects argument of 
type ‘long unsigned int’, but argument 2 has type ‘size_t {aka unsigned 
int}’ [-Werror=format=]

str_value = xasprintf("%lu", s.max_hash_alg);

You'll need to fixup your formatting there.  I don't give 32 bit 
architectures all the attention I should but I suppose they're
still in use.

Thanks,

- Greg

>
> Thanks William!
>
> - Greg
>
>> ---
>> v2:
>>     rebase to master
>> ---
>>   ofproto/ofproto-dpif.c     | 51 +++++++++++++++++++++++++++++++
>>   ofproto/ofproto-provider.h |  2 ++
>>   ofproto/ofproto.c          | 12 ++++++++
>>   ofproto/ofproto.h          |  2 ++
>>   tests/ovs-vsctl.at         | 10 ++++++
>>   utilities/ovs-vsctl.8.in   |  6 ++++
>>   utilities/ovs-vsctl.c      | 28 +++++++++++++++++
>>   vswitchd/bridge.c          | 21 +++++++++++++
>>   vswitchd/vswitch.ovsschema |  5 ++-
>>   vswitchd/vswitch.xml       | 76 
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>   10 files changed, 212 insertions(+), 1 deletion(-)
>>
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index 496a16c8a4af..c37181664523 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> @@ -5440,6 +5440,56 @@ ct_del_zone_timeout_policy(const char 
>> *datapath_type, uint16_t zone_id)
>>       }
>>   }
>>   +static void
>> +get_datapath_cap(const char *datapath_type, struct smap *cap)
>> +{
>> +    char *str_value;
>> +    struct odp_support odp;
>> +    struct dpif_backer_support s;
>> +    struct dpif_backer *backer = shash_find_data(&all_dpif_backers,
>> + datapath_type);
>> +    if (!backer) {
>> +        return;
>> +    }
>> +    s = backer->rt_support;
>> +    odp = s.odp;
>> +
>> +    /* ODP_SUPPORT_FIELDS */
>> +    str_value = xasprintf("%lu", odp.max_vlan_headers);
>> +    smap_add(cap, "max_vlan_headers", str_value);
>> +    free(str_value);
>> +
>> +    str_value = xasprintf("%lu", odp.max_mpls_depth);
>> +    smap_add(cap, "max_mpls_depth", str_value);
>> +    free(str_value);
>> +
>> +    smap_add(cap, "recirc", odp.recirc ? "true" : "false");
>> +    smap_add(cap, "ct_state", odp.ct_state ? "true" : "false");
>> +    smap_add(cap, "ct_zone", odp.ct_zone ? "true" : "false");
>> +    smap_add(cap, "ct_mark", odp.ct_mark ? "true" : "false");
>> +    smap_add(cap, "ct_label", odp.ct_label ? "true" : "false");
>> +    smap_add(cap, "ct_state_nat", odp.ct_state_nat ? "true" : "false");
>> +    smap_add(cap, "ct_orig_tuple", odp.ct_orig_tuple ? "true" : 
>> "false");
>> +    smap_add(cap, "ct_orig_tuple6", odp.ct_orig_tuple6 ? "true" : 
>> "false");
>> +
>> +    /* DPIF_SUPPORT_FIELDS */
>> +    smap_add(cap, "masked_set_action", s.masked_set_action ? "true" 
>> : "false");
>> +    smap_add(cap, "tnl_push_pop", s.tnl_push_pop ? "true" : "false");
>> +    smap_add(cap, "ufid", s.ufid ? "true" : "false");
>> +    smap_add(cap, "trunc", s.trunc ? "true" : "false");
>> +    smap_add(cap, "clone", s.clone ? "true" : "false");
>> +    smap_add(cap, "sample_nesting", s.sample_nesting ? "true" : 
>> "false");
>> +    smap_add(cap, "ct_eventmask", s.ct_eventmask ? "true" : "false");
>> +    smap_add(cap, "ct_clear", s.ct_clear ? "true" : "false");
>> +
>> +    str_value = xasprintf("%lu", s.max_hash_alg);
>> +    smap_add(cap, "max_hash_alg", str_value);
>> +    free(str_value);
>> +
>> +    smap_add(cap, "check_pkt_len", s.check_pkt_len ? "true" : "false");
>> +    smap_add(cap, "ct_timeout", s.ct_timeout ? "true" : "false");
>> +}
>> +
>>   /* Gets timeout policy name in 'backer' based on 'zone', 'dl_type' and
>>    * 'nw_proto'.  Returns true if the zone-based timeout policy is 
>> configured.
>>    * On success, stores the timeout policy name in 'tp_name', and sets
>> @@ -6581,4 +6631,5 @@ const struct ofproto_class ofproto_dpif_class = {
>>       ct_flush,                   /* ct_flush */
>>       ct_set_zone_timeout_policy,
>>       ct_del_zone_timeout_policy,
>> +    get_datapath_cap,
>>   };
>> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
>> index c980e6bffff5..80c4fee06d01 100644
>> --- a/ofproto/ofproto-provider.h
>> +++ b/ofproto/ofproto-provider.h
>> @@ -1889,6 +1889,8 @@ struct ofproto_class {
>>       /* Deletes the timeout policy associated with 'zone' in 
>> datapath type
>>        * 'dp_type'. */
>>       void (*ct_del_zone_timeout_policy)(const char *dp_type, 
>> uint16_t zone);
>> +    /* Get the datapath's capabilities. */
>> +    void (*get_datapath_cap)(const char *dp_type, struct smap *caps);
>>   };
>>     extern const struct ofproto_class ofproto_dpif_class;
>> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
>> index b4249b0d8818..b3d68e74af3f 100644
>> --- a/ofproto/ofproto.c
>> +++ b/ofproto/ofproto.c
>> @@ -954,6 +954,18 @@ ofproto_get_flow_restore_wait(void)
>>       return flow_restore_wait;
>>   }
>>   +/* Retrieve datapath capabilities. */
>> +void
>> +ofproto_get_datapath_cap(const char *datapath_type, struct smap 
>> *dp_cap)
>> +{
>> +    datapath_type = ofproto_normalize_type(datapath_type);
>> +    const struct ofproto_class *class = 
>> ofproto_class_find__(datapath_type);
>> +
>> +    if (class->get_datapath_cap) {
>> +        class->get_datapath_cap(datapath_type, dp_cap);
>> +    }
>> +}
>> +
>>   /* Connection tracking configuration. */
>>   void
>>   ofproto_ct_set_zone_timeout_policy(const char *datapath_type, 
>> uint16_t zone_id,
>> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
>> index 033c4cf93e94..48a9d602c214 100644
>> --- a/ofproto/ofproto.h
>> +++ b/ofproto/ofproto.h
>> @@ -372,6 +372,8 @@ void ofproto_ct_set_zone_timeout_policy(const 
>> char *datapath_type,
>>                                           struct simap *timeout_policy);
>>   void ofproto_ct_del_zone_timeout_policy(const char *datapath_type,
>>                                           uint16_t zone);
>> +void ofproto_get_datapath_cap(const char *datapath_type,
>> +                              struct smap *dp_cap);
>>     /* Configuration of ports. */
>>   void ofproto_port_unregister(struct ofproto *, ofp_port_t ofp_port);
>> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
>> index 4d55f89e4ae3..18b9cd7c89a5 100644
>> --- a/tests/ovs-vsctl.at
>> +++ b/tests/ovs-vsctl.at
>> @@ -819,6 +819,10 @@ AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev 
>> zone=1])])
>>   AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=1])])
>>   AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, 
>> Timeout Policies: icmp_first=2 icmp_reply=3
>>   ])
>> +
>> +AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath 
>> datapath_version=0 'capabilities={recirc=true}' -- set Open_vSwitch . 
>> datapaths:"system"=@m])], [0], [stdout])
>> +AT_CHECK([RUN_OVS_VSCTL([list-dp-cap system])], [0], [recirc=true
>> +])
>>   OVS_VSCTL_CLEANUP
>>   AT_CLEANUP
>>   @@ -962,6 +966,12 @@ AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev 
>> zone=11])],
>>   ])
>>   AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, 
>> Timeout Policies: icmp_first=2 icmp_reply=3
>>   ])
>> +
>> +AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath 
>> datapath_version=0 'capabilities={recirc=true}' -- set Open_vSwitch . 
>> datapaths:"system"=@m])], [0], [stdout])
>> +AT_CHECK([RUN_OVS_VSCTL([list-dp-cap nosystem])],
>> +  [1], [], [ovs-vsctl: datapath "nosystem" record not found
>> +])
>> +
>>   OVS_VSCTL_CLEANUP
>>   AT_CLEANUP
>>   diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
>> index 14a8aa4a48ac..ff97922dd0c6 100644
>> --- a/utilities/ovs-vsctl.8.in
>> +++ b/utilities/ovs-vsctl.8.in
>> @@ -379,6 +379,12 @@ delete a zone that does not exist has no effect.
>>   .IP "\fBlist\-zone\-tp \fIdatapath\fR"
>>   Prints the timeout policies of all zones in \fIdatapath\fR.
>>   .
>> +.SS "Datapath Capabilities Command"
>> +The command query datapath capabilities.
>> +.
>> +.IP "\fBlist\-dp\-cap \fIdatapath\fR"
>> +Prints the datapath's capabilities.
>> +.
>>   .SS "OpenFlow Controller Connectivity"
>>   .
>>   \fBovs\-vswitchd\fR can perform all configured bridging and switching
>> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
>> index 7232471e68b9..bd3972636e66 100644
>> --- a/utilities/ovs-vsctl.c
>> +++ b/utilities/ovs-vsctl.c
>> @@ -1363,6 +1363,31 @@ pre_get_zone(struct ctl_context *ctx)
>>   }
>>     static void
>> +pre_get_dp_cap(struct ctl_context *ctx)
>> +{
>> +    ovsdb_idl_add_column(ctx->idl, &ovsrec_open_vswitch_col_datapaths);
>> +    ovsdb_idl_add_column(ctx->idl, &ovsrec_datapath_col_capabilities);
>> +}
>> +
>> +static void
>> +cmd_list_dp_cap(struct ctl_context *ctx)
>> +{
>> +    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>> +    struct smap_node *node;
>> +
>> +    struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, 
>> ctx->argv[1]);
>> +    if (!dp) {
>> +        ctl_fatal("datapath \"%s\" record not found", ctx->argv[1]);
>> +    }
>> +
>> +    SMAP_FOR_EACH (node, &dp->capabilities) {
>> +        ds_put_format(&ctx->output, "%s=%s ",node->key, node->value);
>> +    }
>> +    ds_chomp(&ctx->output, ' ');
>> +    ds_put_char(&ctx->output, '\n');
>> +}
>> +
>> +static void
>>   cmd_add_br(struct ctl_context *ctx)
>>   {
>>       struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
>> @@ -3112,6 +3137,9 @@ static const struct ctl_command_syntax 
>> vsctl_commands[] = {
>>        "--if-exists", RW},
>>       {"list-zone-tp", 1, 1, "", pre_get_zone, cmd_list_zone_tp, 
>> NULL, "", RO},
>>   +    /* Datapath capabilities. */
>> +    {"list-dp-cap", 1, 1, "", pre_get_dp_cap, cmd_list_dp_cap, NULL, 
>> "", RO},
>> +
>>       {NULL, 0, 0, NULL, NULL, NULL, NULL, NULL, RO},
>>   };
>>   diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>> index 9095ebf5d5e9..d402e39bdc68 100644
>> --- a/vswitchd/bridge.c
>> +++ b/vswitchd/bridge.c
>> @@ -171,6 +171,7 @@ struct datapath {
>>       struct hmap ct_zones;       /* Map of 'struct ct_zone' 
>> elements, indexed
>>                                    * by 'zone'. */
>>       struct hmap_node node;      /* Node in 'all_datapaths' hmap. */
>> +    struct smap caps;           /* Capabilities. */
>>       unsigned int last_used;     /* The last idl_seqno that this 
>> 'datapath'
>>                                    * used in OVSDB. This number is 
>> used for
>>                                    * garbage collection. */
>> @@ -700,6 +701,7 @@ datapath_create(const char *type)
>>       dp->type = xstrdup(type);
>>       hmap_init(&dp->ct_zones);
>>       hmap_insert(&all_datapaths, &dp->node, hash_string(type, 0));
>> +    smap_init(&dp->caps);
>>       return dp;
>>   }
>>   @@ -716,6 +718,7 @@ datapath_destroy(struct datapath *dp)
>>           hmap_remove(&all_datapaths, &dp->node);
>>           hmap_destroy(&dp->ct_zones);
>>           free(dp->type);
>> +        smap_destroy(&dp->caps);
>>           free(dp);
>>       }
>>   }
>> @@ -759,6 +762,23 @@ ct_zones_reconfigure(struct datapath *dp, struct 
>> ovsrec_datapath *dp_cfg)
>>   }
>>     static void
>> +dp_capability_reconfigure(struct datapath *dp,
>> +                          struct ovsrec_datapath *dp_cfg)
>> +{
>> +    struct smap_node *node;
>> +    struct smap cap;
>> +
>> +    smap_init(&cap);
>> +    ofproto_get_datapath_cap(dp->type, &cap);
>> +
>> +    SMAP_FOR_EACH (node, &cap) {
>> +        ovsrec_datapath_update_capabilities_setkey(dp_cfg, node->key,
>> + node->value);
>> +    }
>> +    smap_destroy(&cap);
>> +}
>> +
>> +static void
>>   datapath_reconfigure(const struct ovsrec_open_vswitch *cfg)
>>   {
>>       struct datapath *dp, *next;
>> @@ -771,6 +791,7 @@ datapath_reconfigure(const struct 
>> ovsrec_open_vswitch *cfg)
>>           dp = datapath_lookup(dp_name);
>>           if (!dp) {
>>               dp = datapath_create(dp_name);
>> +            dp_capability_reconfigure(dp, dp_cfg);
>>           }
>>           dp->last_used = idl_seqno;
>>           ct_zones_reconfigure(dp, dp_cfg);
>> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
>> index 02be5ddeec97..0666c8c76446 100644
>> --- a/vswitchd/vswitch.ovsschema
>> +++ b/vswitchd/vswitch.ovsschema
>> @@ -1,6 +1,6 @@
>>   {"name": "Open_vSwitch",
>>    "version": "8.2.0",
>> - "cksum": "4076590391 26298",
>> + "cksum": "1076640191 26427",
>>    "tables": {
>>      "Open_vSwitch": {
>>        "columns": {
>> @@ -650,6 +650,9 @@
>>                     "value": {"type": "uuid",
>>                               "refTable": "CT_Zone"},
>>                     "min": 0, "max": "unlimited"}},
>> +       "capabilities": {
>> +         "type": {"key": "string", "value": "string",
>> +                  "min": 0, "max": "unlimited"}},
>>          "external_ids": {
>>            "type": {"key": "string", "value": "string",
>>                     "min": 0, "max": "unlimited"}}}},
>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> index 01304a5ed2c2..6201f5fd7886 100644
>> --- a/vswitchd/vswitch.xml
>> +++ b/vswitchd/vswitch.xml
>> @@ -5650,6 +5650,82 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 
>> type=patch options:peer=p1 \
>>         connection tracking-related OpenFlow matches and actions).
>>       </column>
>>   +    <column name="capabilities" key="max_vlan_headers"
>> +            type='{"type": "integer", "minInteger": 0}'>
>> +      Maximum number of 802.1q VLAN headers to serialize in a mask.
>> +    </column>
>> +    <column name="capabilities" key="max_mpls_depth"
>> +            type='{"type": "integer", "minInteger": 0}'>
>> +      Maximum number of MPLS label stack entries to serialise in a 
>> mask.
>> +    </column>
>> +    <column name="capabilities" key="recirc" type='{"type": 
>> "boolean"}'>
>> +      If this is true, then recirculation fields will always be 
>> serialised.
>> +    </column>
>> +    <column name="capabilities" key="ct_state" type='{"type": 
>> "boolean"}'>
>> +      If true, datapath supports OVS_KEY_ATTR_CT_STATE.
>> +    </column>
>> +    <column name="capabilities" key="ct_zone" type='{"type": 
>> "boolean"}'>
>> +      If true, datapath supports OVS_KEY_ATTR_CT_ZONE.
>> +    </column>
>> +    <column name="capabilities" key="ct_mark" type='{"type": 
>> "boolean"}'>
>> +      If true, datapath supports OVS_KEY_ATTR_CT_MARK.
>> +    </column>
>> +    <column name="capabilities" key="ct_label" type='{"type": 
>> "boolean"}'>
>> +      If true, datapath supports OVS_KEY_ATTR_CT_LABEL.
>> +    </column>
>> +    <column name="capabilities" key="ct_state_nat" type='{"type": 
>> "boolean"}'>
>> +      If true, it means that the datapath supports the NAT bits in
>> +      'ct_state'.  The above 'ct_state' member must be true for this
>> +      to make sense.
>> +    </column>
>> +    <column name="capabilities" key="ct_orig_tuple"
>> +            type='{"type": "boolean"}'>
>> +      Conntrack original direction tuple matching * supported.
>> +    </column>
>> +    <column name="capabilities" key="ct_orig_tuple6"
>> +            type='{"type": "boolean"}'>
>> +      Conntrack original direction tuple6 matching * supported.
>> +    </column>
>> +    <column name="capabilities" key="masked_set_action"
>> +            type='{"type": "boolean"}'>
>> +      True if the datapath supports masked data in
>> +      OVS_ACTION_ATTR_SET actions.
>> +    </column>
>> +    <column name="capabilities" key="tnl_push_pop" type='{"type": 
>> "boolean"}'>
>> +      True if the datapath supports tnl_push and pop actions.
>> +    </column>
>> +    <column name="capabilities" key="ufid" type='{"type": "boolean"}'>
>> +      True if the datapath supports OVS_FLOW_ATTR_UFID.
>> +    </column>
>> +    <column name="capabilities" key="trunc" type='{"type": "boolean"}'>
>> +      True if the datapath supports OVS_ACTION_ATTR_TRUNC action.
>> +    </column>
>> +    <column name="capabilities" key="clone" type='{"type": "boolean"}'>
>> +      True if the datapath supports OVS_ACTION_ATTR_CLONE action.
>> +    </column>
>> +    <column name="capabilities" key="sample_nesting"
>> +            type='{"type": "integer", "minInteger": 0}'>
>> +      Maximum level of nesting allowed by OVS_ACTION_ATTR_SAMPLE 
>> action.
>> +    </column>
>> +    <column name="capabilities" key="ct_eventmask" type='{"type": 
>> "boolean"}'>
>> +      OVS_CT_ATTR_EVENTMASK supported by OVS_ACTION_ATTR_CT action.
>> +    </column>
>> +    <column name="capabilities" key="ct_clear" type='{"type": 
>> "boolean"}'>
>> +      True if the datapath supports OVS_ACTION_ATTR_CT_CLEAR action.
>> +    </column>
>> +    <column name="capabilities" key="max_hash_alg"
>> +            type='{"type": "integer", "minInteger": 0}'>
>> +      Highest supported dp_hash algorithm.
>> +    </column>
>> +    <column name="capabilities" key="check_pkt_len"
>> +            type='{"type": "boolean"}'>
>> +      True if the datapath supports OVS_ACTION_ATTR_CHECK_PKT_LEN.
>> +    </column>
>> +    <column name="capabilities" key="ct_timeout" type='{"type": 
>> "boolean"}'>
>> +      True if the datapath supports OVS_CT_ATTR_TIMEOUT in
>> +      OVS_ACTION_ATTR_CTaction.
>> +    </column>
>> +
>>       <group title="Common Columns">
>>         The overall purpose of these columns is described under 
>> <code>Common
>>         Columns</code> at the beginning of this document.
>
William Tu Oct. 17, 2019, 4:27 p.m. UTC | #3
On Wed, Oct 16, 2019 at 9:36 AM Gregory Rose <gvrose8192@gmail.com> wrote:
>
>
> On 10/15/2019 2:06 PM, Gregory Rose wrote:
>
>
> On 10/4/2019 3:32 PM, William Tu wrote:
>
> The patch adds support for fetching the datapath's capabilities
> from the result of 'check_support()', and write the supported capability
> to a new database column, called 'capabilities' under Datapath table.
>
> To see how it works, run:
>   # ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
>   # ovs-vsctl -- --id=@m create Datapath datapath_version=0 \
>       'ct_zones={}' 'capabilities={}' \
>       -- set Open_vSwitch . datapaths:"netdev"=@m
>
>   # ovs-vsctl list-dp-cap netdev
>   ufid=true sample_nesting=true clone=true tnl_push_pop=true \
>   ct_orig_tuple=true ct_eventmask=true ct_state=true \
>   ct_clear=true max_vlan_headers=1 recirc=true ct_label=true \
>   max_hash_alg=1 ct_state_nat=true ct_timeout=true \
>   ct_mark=true ct_orig_tuple6=true check_pkt_len=true \
>   masked_set_action=true max_mpls_depth=3 trunc=true ct_zone=true
>
> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/593749381
> Signed-off-by: William Tu <u9012063@gmail.com>
>
>
> Patch works as advertised and the code looks fine to me.
>
> Tested-by: Greg Rose <gvrose8192@gmail.com>
> Reviewed-by: Greg Rose <gvrose8192@gmail.com>
>
>
> I have to take this back.  While doing other work I found that this patch has an issue on 32bit
> architecture:
>
> ofproto/ofproto-dpif.c: In function ‘get_datapath_cap’:
>
> ofproto/ofproto-dpif.c:5458:27: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘size_t {aka unsigned int}’ [-Werror=format=]
>
>      str_value = xasprintf("%lu", odp.max_vlan_headers);
>
>                            ^
>
> ofproto/ofproto-dpif.c:5462:27: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘size_t {aka unsigned int}’ [-Werror=format=]
>
>      str_value = xasprintf("%lu", odp.max_mpls_depth);
>
>                            ^
>
> ofproto/ofproto-dpif.c:5485:27: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘size_t {aka unsigned int}’ [-Werror=format=]
>
>      str_value = xasprintf("%lu", s.max_hash_alg);
>
> You'll need to fixup your formatting there.  I don't give 32 bit architectures all the attention I should but I suppose they're
> still in use.
>

Thanks Greg!

I kept making the same mistake. I will fix it next version.

William
diff mbox series

Patch

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 496a16c8a4af..c37181664523 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5440,6 +5440,56 @@  ct_del_zone_timeout_policy(const char *datapath_type, uint16_t zone_id)
     }
 }
 
+static void
+get_datapath_cap(const char *datapath_type, struct smap *cap)
+{
+    char *str_value;
+    struct odp_support odp;
+    struct dpif_backer_support s;
+    struct dpif_backer *backer = shash_find_data(&all_dpif_backers,
+                                                 datapath_type);
+    if (!backer) {
+        return;
+    }
+    s = backer->rt_support;
+    odp = s.odp;
+
+    /* ODP_SUPPORT_FIELDS */
+    str_value = xasprintf("%lu", odp.max_vlan_headers);
+    smap_add(cap, "max_vlan_headers", str_value);
+    free(str_value);
+
+    str_value = xasprintf("%lu", odp.max_mpls_depth);
+    smap_add(cap, "max_mpls_depth", str_value);
+    free(str_value);
+
+    smap_add(cap, "recirc", odp.recirc ? "true" : "false");
+    smap_add(cap, "ct_state", odp.ct_state ? "true" : "false");
+    smap_add(cap, "ct_zone", odp.ct_zone ? "true" : "false");
+    smap_add(cap, "ct_mark", odp.ct_mark ? "true" : "false");
+    smap_add(cap, "ct_label", odp.ct_label ? "true" : "false");
+    smap_add(cap, "ct_state_nat", odp.ct_state_nat ? "true" : "false");
+    smap_add(cap, "ct_orig_tuple", odp.ct_orig_tuple ? "true" : "false");
+    smap_add(cap, "ct_orig_tuple6", odp.ct_orig_tuple6 ? "true" : "false");
+
+    /* DPIF_SUPPORT_FIELDS */
+    smap_add(cap, "masked_set_action", s.masked_set_action ? "true" : "false");
+    smap_add(cap, "tnl_push_pop", s.tnl_push_pop ? "true" : "false");
+    smap_add(cap, "ufid", s.ufid ? "true" : "false");
+    smap_add(cap, "trunc", s.trunc ? "true" : "false");
+    smap_add(cap, "clone", s.clone ? "true" : "false");
+    smap_add(cap, "sample_nesting", s.sample_nesting ? "true" : "false");
+    smap_add(cap, "ct_eventmask", s.ct_eventmask ? "true" : "false");
+    smap_add(cap, "ct_clear", s.ct_clear ? "true" : "false");
+
+    str_value = xasprintf("%lu", s.max_hash_alg);
+    smap_add(cap, "max_hash_alg", str_value);
+    free(str_value);
+
+    smap_add(cap, "check_pkt_len", s.check_pkt_len ? "true" : "false");
+    smap_add(cap, "ct_timeout", s.ct_timeout ? "true" : "false");
+}
+
 /* Gets timeout policy name in 'backer' based on 'zone', 'dl_type' and
  * 'nw_proto'.  Returns true if the zone-based timeout policy is configured.
  * On success, stores the timeout policy name in 'tp_name', and sets
@@ -6581,4 +6631,5 @@  const struct ofproto_class ofproto_dpif_class = {
     ct_flush,                   /* ct_flush */
     ct_set_zone_timeout_policy,
     ct_del_zone_timeout_policy,
+    get_datapath_cap,
 };
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index c980e6bffff5..80c4fee06d01 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1889,6 +1889,8 @@  struct ofproto_class {
     /* Deletes the timeout policy associated with 'zone' in datapath type
      * 'dp_type'. */
     void (*ct_del_zone_timeout_policy)(const char *dp_type, uint16_t zone);
+    /* Get the datapath's capabilities. */
+    void (*get_datapath_cap)(const char *dp_type, struct smap *caps);
 };
 
 extern const struct ofproto_class ofproto_dpif_class;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index b4249b0d8818..b3d68e74af3f 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -954,6 +954,18 @@  ofproto_get_flow_restore_wait(void)
     return flow_restore_wait;
 }
 
+/* Retrieve datapath capabilities. */
+void
+ofproto_get_datapath_cap(const char *datapath_type, struct smap *dp_cap)
+{
+    datapath_type = ofproto_normalize_type(datapath_type);
+    const struct ofproto_class *class = ofproto_class_find__(datapath_type);
+
+    if (class->get_datapath_cap) {
+        class->get_datapath_cap(datapath_type, dp_cap);
+    }
+}
+
 /* Connection tracking configuration. */
 void
 ofproto_ct_set_zone_timeout_policy(const char *datapath_type, uint16_t zone_id,
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 033c4cf93e94..48a9d602c214 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -372,6 +372,8 @@  void ofproto_ct_set_zone_timeout_policy(const char *datapath_type,
                                         struct simap *timeout_policy);
 void ofproto_ct_del_zone_timeout_policy(const char *datapath_type,
                                         uint16_t zone);
+void ofproto_get_datapath_cap(const char *datapath_type,
+                              struct smap *dp_cap);
 
 /* Configuration of ports. */
 void ofproto_port_unregister(struct ofproto *, ofp_port_t ofp_port);
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 4d55f89e4ae3..18b9cd7c89a5 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -819,6 +819,10 @@  AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
 AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=1])])
 AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
 ])
+
+AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 'capabilities={recirc=true}' -- set Open_vSwitch . datapaths:"system"=@m])], [0], [stdout])
+AT_CHECK([RUN_OVS_VSCTL([list-dp-cap system])], [0], [recirc=true
+])
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
@@ -962,6 +966,12 @@  AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=11])],
 ])
 AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
 ])
+
+AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 'capabilities={recirc=true}' -- set Open_vSwitch . datapaths:"system"=@m])], [0], [stdout])
+AT_CHECK([RUN_OVS_VSCTL([list-dp-cap nosystem])],
+  [1], [], [ovs-vsctl: datapath "nosystem" record not found
+])
+
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 14a8aa4a48ac..ff97922dd0c6 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -379,6 +379,12 @@  delete a zone that does not exist has no effect.
 .IP "\fBlist\-zone\-tp \fIdatapath\fR"
 Prints the timeout policies of all zones in \fIdatapath\fR.
 .
+.SS "Datapath Capabilities Command"
+The command query datapath capabilities.
+.
+.IP "\fBlist\-dp\-cap \fIdatapath\fR"
+Prints the datapath's capabilities.
+.
 .SS "OpenFlow Controller Connectivity"
 .
 \fBovs\-vswitchd\fR can perform all configured bridging and switching
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 7232471e68b9..bd3972636e66 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -1363,6 +1363,31 @@  pre_get_zone(struct ctl_context *ctx)
 }
 
 static void
+pre_get_dp_cap(struct ctl_context *ctx)
+{
+    ovsdb_idl_add_column(ctx->idl, &ovsrec_open_vswitch_col_datapaths);
+    ovsdb_idl_add_column(ctx->idl, &ovsrec_datapath_col_capabilities);
+}
+
+static void
+cmd_list_dp_cap(struct ctl_context *ctx)
+{
+    struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
+    struct smap_node *node;
+
+    struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, ctx->argv[1]);
+    if (!dp) {
+        ctl_fatal("datapath \"%s\" record not found", ctx->argv[1]);
+    }
+
+    SMAP_FOR_EACH (node, &dp->capabilities) {
+        ds_put_format(&ctx->output, "%s=%s ",node->key, node->value);
+    }
+    ds_chomp(&ctx->output, ' ');
+    ds_put_char(&ctx->output, '\n');
+}
+
+static void
 cmd_add_br(struct ctl_context *ctx)
 {
     struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
@@ -3112,6 +3137,9 @@  static const struct ctl_command_syntax vsctl_commands[] = {
      "--if-exists", RW},
     {"list-zone-tp", 1, 1, "", pre_get_zone, cmd_list_zone_tp, NULL, "", RO},
 
+    /* Datapath capabilities. */
+    {"list-dp-cap", 1, 1, "", pre_get_dp_cap, cmd_list_dp_cap, NULL, "", RO},
+
     {NULL, 0, 0, NULL, NULL, NULL, NULL, NULL, RO},
 };
 
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 9095ebf5d5e9..d402e39bdc68 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -171,6 +171,7 @@  struct datapath {
     struct hmap ct_zones;       /* Map of 'struct ct_zone' elements, indexed
                                  * by 'zone'. */
     struct hmap_node node;      /* Node in 'all_datapaths' hmap. */
+    struct smap caps;           /* Capabilities. */
     unsigned int last_used;     /* The last idl_seqno that this 'datapath'
                                  * used in OVSDB. This number is used for
                                  * garbage collection. */
@@ -700,6 +701,7 @@  datapath_create(const char *type)
     dp->type = xstrdup(type);
     hmap_init(&dp->ct_zones);
     hmap_insert(&all_datapaths, &dp->node, hash_string(type, 0));
+    smap_init(&dp->caps);
     return dp;
 }
 
@@ -716,6 +718,7 @@  datapath_destroy(struct datapath *dp)
         hmap_remove(&all_datapaths, &dp->node);
         hmap_destroy(&dp->ct_zones);
         free(dp->type);
+        smap_destroy(&dp->caps);
         free(dp);
     }
 }
@@ -759,6 +762,23 @@  ct_zones_reconfigure(struct datapath *dp, struct ovsrec_datapath *dp_cfg)
 }
 
 static void
+dp_capability_reconfigure(struct datapath *dp,
+                          struct ovsrec_datapath *dp_cfg)
+{
+    struct smap_node *node;
+    struct smap cap;
+
+    smap_init(&cap);
+    ofproto_get_datapath_cap(dp->type, &cap);
+
+    SMAP_FOR_EACH (node, &cap) {
+        ovsrec_datapath_update_capabilities_setkey(dp_cfg, node->key,
+                                                   node->value);
+    }
+    smap_destroy(&cap);
+}
+
+static void
 datapath_reconfigure(const struct ovsrec_open_vswitch *cfg)
 {
     struct datapath *dp, *next;
@@ -771,6 +791,7 @@  datapath_reconfigure(const struct ovsrec_open_vswitch *cfg)
         dp = datapath_lookup(dp_name);
         if (!dp) {
             dp = datapath_create(dp_name);
+            dp_capability_reconfigure(dp, dp_cfg);
         }
         dp->last_used = idl_seqno;
         ct_zones_reconfigure(dp, dp_cfg);
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 02be5ddeec97..0666c8c76446 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@ 
 {"name": "Open_vSwitch",
  "version": "8.2.0",
- "cksum": "4076590391 26298",
+ "cksum": "1076640191 26427",
  "tables": {
    "Open_vSwitch": {
      "columns": {
@@ -650,6 +650,9 @@ 
                   "value": {"type": "uuid",
                             "refTable": "CT_Zone"},
                   "min": 0, "max": "unlimited"}},
+       "capabilities": {
+         "type": {"key": "string", "value": "string",
+                  "min": 0, "max": "unlimited"}},
        "external_ids": {
          "type": {"key": "string", "value": "string",
                   "min": 0, "max": "unlimited"}}}},
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 01304a5ed2c2..6201f5fd7886 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -5650,6 +5650,82 @@  ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
       connection tracking-related OpenFlow matches and actions).
     </column>
 
+    <column name="capabilities" key="max_vlan_headers"
+            type='{"type": "integer", "minInteger": 0}'>
+      Maximum number of 802.1q VLAN headers to serialize in a mask.
+    </column>
+    <column name="capabilities" key="max_mpls_depth"
+            type='{"type": "integer", "minInteger": 0}'>
+      Maximum number of MPLS label stack entries to serialise in a mask.
+    </column>
+    <column name="capabilities" key="recirc" type='{"type": "boolean"}'>
+      If this is true, then recirculation fields will always be serialised.
+    </column>
+    <column name="capabilities" key="ct_state" type='{"type": "boolean"}'>
+      If true, datapath supports OVS_KEY_ATTR_CT_STATE.
+    </column>
+    <column name="capabilities" key="ct_zone" type='{"type": "boolean"}'>
+      If true, datapath supports OVS_KEY_ATTR_CT_ZONE.
+    </column>
+    <column name="capabilities" key="ct_mark" type='{"type": "boolean"}'>
+      If true, datapath supports OVS_KEY_ATTR_CT_MARK.
+    </column>
+    <column name="capabilities" key="ct_label" type='{"type": "boolean"}'>
+      If true, datapath supports OVS_KEY_ATTR_CT_LABEL.
+    </column>
+    <column name="capabilities" key="ct_state_nat" type='{"type": "boolean"}'>
+      If true, it means that the datapath supports the NAT bits in
+      'ct_state'.  The above 'ct_state' member must be true for this
+      to make sense.
+    </column>
+    <column name="capabilities" key="ct_orig_tuple"
+            type='{"type": "boolean"}'>
+      Conntrack original direction tuple matching * supported.
+    </column>
+    <column name="capabilities" key="ct_orig_tuple6"
+            type='{"type": "boolean"}'>
+      Conntrack original direction tuple6 matching * supported.
+    </column>
+    <column name="capabilities" key="masked_set_action"
+            type='{"type": "boolean"}'>
+      True if the datapath supports masked data in
+      OVS_ACTION_ATTR_SET actions.
+    </column>
+    <column name="capabilities" key="tnl_push_pop" type='{"type": "boolean"}'>
+      True if the datapath supports tnl_push and pop actions.
+    </column>
+    <column name="capabilities" key="ufid" type='{"type": "boolean"}'>
+      True if the datapath supports OVS_FLOW_ATTR_UFID.
+    </column>
+    <column name="capabilities" key="trunc" type='{"type": "boolean"}'>
+      True if the datapath supports OVS_ACTION_ATTR_TRUNC action.
+    </column>
+    <column name="capabilities" key="clone" type='{"type": "boolean"}'>
+      True if the datapath supports OVS_ACTION_ATTR_CLONE action.
+    </column>
+    <column name="capabilities" key="sample_nesting"
+            type='{"type": "integer", "minInteger": 0}'>
+      Maximum level of nesting allowed by OVS_ACTION_ATTR_SAMPLE action.
+    </column>
+    <column name="capabilities" key="ct_eventmask" type='{"type": "boolean"}'>
+      OVS_CT_ATTR_EVENTMASK supported by OVS_ACTION_ATTR_CT action.
+    </column>
+    <column name="capabilities" key="ct_clear" type='{"type": "boolean"}'>
+      True if the datapath supports OVS_ACTION_ATTR_CT_CLEAR action.
+    </column>
+    <column name="capabilities" key="max_hash_alg"
+            type='{"type": "integer", "minInteger": 0}'>
+      Highest supported dp_hash algorithm.
+    </column>
+    <column name="capabilities" key="check_pkt_len"
+            type='{"type": "boolean"}'>
+      True if the datapath supports OVS_ACTION_ATTR_CHECK_PKT_LEN.
+    </column>
+    <column name="capabilities" key="ct_timeout" type='{"type": "boolean"}'>
+      True if the datapath supports OVS_CT_ATTR_TIMEOUT in
+      OVS_ACTION_ATTR_CTaction.
+    </column>
+
     <group title="Common Columns">
       The overall purpose of these columns is described under <code>Common
       Columns</code> at the beginning of this document.