Re: [PATCH net-next 1/3] net: sched: add mpls manipulation actions to TC

[Davide Caratti <dcaratti@redhat.com>]

hello John!

On Wed, 2019-06-12 at 13:51 +0100, John Hurley wrote:
> Currently, TC offers the ability to match on the MPLS fields of a packet
> through the use of the flow_dissector_key_mpls struct. However, as yet, TC
> actions do not allow the modification or manipulation of such fields.
> 
> Add a new module that registers TC action ops to allow manipulation of
> MPLS. This includes the ability to push and pop headers as well as modify
> the contents of new or existing headers. A further action to decrement the
> TTL field of an MPLS header is also provided.
> 
> Signed-off-by: John Hurley <john.hurley@netronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>

[...]

> index a93680f..197621a 100644
> --- a/include/uapi/linux/pkt_cls.h
> +++ b/include/uapi/linux/pkt_cls.h
> @@ -83,6 +83,7 @@ enum {
>  #define TCA_ACT_SIMP 22
>  #define TCA_ACT_IFE 25
>  #define TCA_ACT_SAMPLE 26
> +#define TCA_ACT_MPLS 27

like I mentioned in my reply to "[PATCH net-next 1/3] net/sched: Introduce
action ct", I think that 27 is forbidden on net-next: this number is
already used in the uAPI for TCA_ID_CTINFO (see below). Like suggested in
the comment above the definition of TCA_ACT_GACT, it's sufficient to add
TCA_ID_MPLS in the enum below.

>  /* Action type identifiers*/
>  enum tca_id {
> @@ -104,6 +105,7 @@ enum tca_id {
>  	TCA_ID_SIMP = TCA_ACT_SIMP,
>  	TCA_ID_IFE = TCA_ACT_IFE,
>  	TCA_ID_SAMPLE = TCA_ACT_SAMPLE,
> +	TCA_ID_MPLS = TCA_ACT_MPLS,
>  	/* other actions go here */
>  	TCA_ID_CTINFO,
>  	__TCA_ID_MAX = 255

and the line that adds TCA_ID_MPLS to enum tca_id should be placed right
before __TCA_ID_MAX, so that the uAPI is preserved (i.e. the value of
TCA_ID_CTINFO does not change).

thanks!
-- 
davide