On Fri, Nov 08, 2019 at 05:12:55PM +0100, Matteo Croce wrote:
> Hi,
>
> I need to add a field to enum ovs_action_attr, but I see that the
> definition between the upstream header[1] and the one in compat[2]
> differs.
> Upstream enum stops at OVS_ACTION_ATTR_CHECK_PKT_LEN, with an extra
> "hidden" element after __OVS_ACTION_ATTR_MAX (22)
> Our compat version instead, has OVS_ACTION_ATTR_TUNNEL_{PUSH,POP}
> defined only #ifndef __KERNEL__, with __OVS_ACTION_ATTR_MAX being 22
> for the kernel and 24 for userspace.
>
> If I add a field OVS_ACTION_ATTR_WHATEVER just before
> __OVS_ACTION_ATTR_MAX in the kernel, older userspace will incorrectly
> see the new action as OVS_ACTION_ATTR_TUNNEL_PUSH.
"older userspace" means you're using userspace datapath (dpif-netdev)?
If true, then it's not using kernel module.
if "older userspace" means just ovs-vswitchd using kernel module,
and you want to upgrade ovs kernel module with your new action
and without upgrade ovs-vswitchd?
Usually we also upgrade ovs-vswitchd, so I don't know how this can be done.
Regards,
William
>
> How can we extend this enum without breaking compatibility?
> If OVS_ACTION_ATTR_TUNNEL_* really is unused in the kernel datapath,
> what if we pad the kernel header with two padding fields?
>
> -----------------------------%<-----------------------------
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -925,6 +926,8 @@ enum ovs_action_attr {
> OVS_ACTION_ATTR_METER, /* u32 meter ID. */
> OVS_ACTION_ATTR_CLONE, /* Nested OVS_CLONE_ATTR_*. */
> OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
> + _OVS_ACTION_ATTR_TUNNEL_PUSH, /* unused in kernel datapath. */
> + _OVS_ACTION_ATTR_TUNNEL_POP, /* unused in kernel datapath. */
>
> __OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted
> ----------------------------->%-----------------------------
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/openvswitch.h?h=v5.3#n899
> [2]
> https://github.com/openvswitch/ovs/blob/v2.12.0/datapath/linux/compat/include/linux/openvswitch.h#L962
>
> Regards,
> --
> Matteo Croce
> per aspera ad upstream
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
