diff mbox

[iproute2,V2,1/2] tc/cls_flower: Classify packet in ip tunnels

Message ID 20161128125136.3393-2-amir@vadai.me
State Changes Requested, archived
Delegated to: stephen hemminger
Headers show

Commit Message

Amir Vadai Nov. 28, 2016, 12:51 p.m. UTC
Introduce classifying by metadata extracted by the tunnel device.
Outer header fields - source/dest ip and tunnel id, are extracted from
the metadata when classifying.

For example, the following will add a filter on the ingress Qdisc of shared
vxlan device named 'vxlan0'. To forward packets with outer src ip
11.11.0.2, dst ip 11.11.0.1 and tunnel id 11. The packets will be
forwarded to tap device 'vnet0':

$ tc filter add dev vxlan0 protocol ip parent ffff: \
    flower \
      enc_src_ip 11.11.0.2 \
      enc_dst_ip 11.11.0.1 \
      enc_key_id 11 \
      dst_ip 11.11.11.1 \
    action mirred egress redirect dev vnet0

Signed-off-by: Amir Vadai <amir@vadai.me>
---
 man/man8/tc-flower.8 | 17 ++++++++++-
 tc/f_flower.c        | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 98 insertions(+), 4 deletions(-)

Comments

Stephen Hemminger Nov. 30, 2016, 3:26 a.m. UTC | #1
The overall design is fine, just a couple nits with the code.

> diff --git a/tc/f_flower.c b/tc/f_flower.c
> index 2d31d1aa832d..1cf0750b5b83 100644
> --- a/tc/f_flower.c
> +++ b/tc/f_flower.c

>  
> +static int flower_parse_key_id(char *str, int type, struct nlmsghdr *n)

str is not modified, therefore use: const char *str

> +{
> +	int ret;
> +	__be32 key_id;
> +
> +	ret = get_be32(&key_id, str, 10);
> +	if (ret)
> +		return -1;

Traditionally netlink attributes are in host order, why was flower
chosen to be different?

> +
> +	addattr32(n, MAX_MSG, type, key_id);
> +
> +	return 0;


Why lose the return value here?  Why not:

	ret = get_be32(&key_id, str, 10);
	if (!ret)
		addattr32(n, MAX_MSG, type, key_id);

> +}
> +
>  static int flower_parse_opt(struct filter_util *qu, char *handle,
>  			    int argc, char **argv, struct nlmsghdr *n)
>  {
> @@ -339,6 +359,38 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
>  				fprintf(stderr, "Illegal \"src_port\"\n");
>  				return -1;
>  			}
> +		} else if (matches(*argv, "enc_dst_ip") == 0) {
> +			NEXT_ARG();
> +			ret = flower_parse_ip_addr(*argv, 0,
> +						   TCA_FLOWER_KEY_ENC_IPV4_DST,
> +						   TCA_FLOWER_KEY_ENC_IPV4_DST_MASK,
> +						   TCA_FLOWER_KEY_ENC_IPV6_DST,
> +						   TCA_FLOWER_KEY_ENC_IPV6_DST_MASK,
> +						   n);
> +			if (ret < 0) {
> +				fprintf(stderr, "Illegal \"enc_dst_ip\"\n");
> +				return -1;
> +			}
> +		} else if (matches(*argv, "enc_src_ip") == 0) {
> +			NEXT_ARG();
> +			ret = flower_parse_ip_addr(*argv, 0,
> +						   TCA_FLOWER_KEY_ENC_IPV4_SRC,
> +						   TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK,
> +						   TCA_FLOWER_KEY_ENC_IPV6_SRC,
> +						   TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK,
> +						   n);
> +			if (ret < 0) {
> +				fprintf(stderr, "Illegal \"enc_src_ip\"\n");
> +				return -1;
> +			}
> +		} else if (matches(*argv, "enc_key_id") == 0) {
> +			NEXT_ARG();
> +			ret = flower_parse_key_id(*argv,
> +						  TCA_FLOWER_KEY_ENC_KEY_ID, n);
> +			if (ret < 0) {
> +				fprintf(stderr, "Illegal \"enc_key_id\"\n");
> +				return -1;
> +			}
>  		} else if (matches(*argv, "action") == 0) {
>  			NEXT_ARG();
>  			ret = parse_action(&argc, &argv, TCA_FLOWER_ACT, n);
> @@ -509,6 +561,14 @@ static void flower_print_port(FILE *f, char *name, __u8 ip_proto,
>  	fprintf(f, "\n  %s %d", name, ntohs(rta_getattr_u16(attr)));
>  }
>  
> +static void flower_print_key_id(FILE *f, char *name,
> +				struct rtattr *attr)

const char *name?


> +{
> +	if (!attr)
> +		return;
> +	fprintf(f, "\n  %s %d", name, ntohl(rta_getattr_u32(attr)));
> +}
> +

Why short circuit, just change the order:

	if (attr)
		fprintf(f, "\n  %s %s", name, ntohl(rta_getattr_u32(attr));

You might also want to introduce rta_getattr_be32()

Please change, retest and resubmit both patches.
Amir Vadai Nov. 30, 2016, 7:17 a.m. UTC | #2
On Tue, Nov 29, 2016 at 07:26:58PM -0800, Stephen Hemminger wrote:
> The overall design is fine, just a couple nits with the code.
> 
> > diff --git a/tc/f_flower.c b/tc/f_flower.c
> > index 2d31d1aa832d..1cf0750b5b83 100644
> > --- a/tc/f_flower.c
> > +++ b/tc/f_flower.c
> 
> >  
> > +static int flower_parse_key_id(char *str, int type, struct nlmsghdr *n)
> 
> str is not modified, therefore use: const char *str
ack

> 
> > +{
> > +	int ret;
> > +	__be32 key_id;
> > +
> > +	ret = get_be32(&key_id, str, 10);
> > +	if (ret)
> > +		return -1;
> 
> Traditionally netlink attributes are in host order, why was flower
> chosen to be different?
I don't know, maybe Jiri (cc'ed) can explain, but it is all over the
flower code.

> 
> > +
> > +	addattr32(n, MAX_MSG, type, key_id);
> > +
> > +	return 0;
> 
> 
> Why lose the return value here?  Why not:
> 
> 	ret = get_be32(&key_id, str, 10);
> 	if (!ret)
> 		addattr32(n, MAX_MSG, type, key_id);
The get_*() function can return only -1 or 0. But you are right, and it
is better the way you suggested.  Changing accordingly in V3.

> 
> > +}
> > +
> >  static int flower_parse_opt(struct filter_util *qu, char *handle,
> >  			    int argc, char **argv, struct nlmsghdr *n)
> >  {
> > @@ -339,6 +359,38 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
> >  				fprintf(stderr, "Illegal \"src_port\"\n");
> >  				return -1;
> >  			}
> > +		} else if (matches(*argv, "enc_dst_ip") == 0) {
> > +			NEXT_ARG();
> > +			ret = flower_parse_ip_addr(*argv, 0,
> > +						   TCA_FLOWER_KEY_ENC_IPV4_DST,
> > +						   TCA_FLOWER_KEY_ENC_IPV4_DST_MASK,
> > +						   TCA_FLOWER_KEY_ENC_IPV6_DST,
> > +						   TCA_FLOWER_KEY_ENC_IPV6_DST_MASK,
> > +						   n);
> > +			if (ret < 0) {
> > +				fprintf(stderr, "Illegal \"enc_dst_ip\"\n");
> > +				return -1;
> > +			}
> > +		} else if (matches(*argv, "enc_src_ip") == 0) {
> > +			NEXT_ARG();
> > +			ret = flower_parse_ip_addr(*argv, 0,
> > +						   TCA_FLOWER_KEY_ENC_IPV4_SRC,
> > +						   TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK,
> > +						   TCA_FLOWER_KEY_ENC_IPV6_SRC,
> > +						   TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK,
> > +						   n);
> > +			if (ret < 0) {
> > +				fprintf(stderr, "Illegal \"enc_src_ip\"\n");
> > +				return -1;
> > +			}
> > +		} else if (matches(*argv, "enc_key_id") == 0) {
> > +			NEXT_ARG();
> > +			ret = flower_parse_key_id(*argv,
> > +						  TCA_FLOWER_KEY_ENC_KEY_ID, n);
> > +			if (ret < 0) {
> > +				fprintf(stderr, "Illegal \"enc_key_id\"\n");
> > +				return -1;
> > +			}
> >  		} else if (matches(*argv, "action") == 0) {
> >  			NEXT_ARG();
> >  			ret = parse_action(&argc, &argv, TCA_FLOWER_ACT, n);
> > @@ -509,6 +561,14 @@ static void flower_print_port(FILE *f, char *name, __u8 ip_proto,
> >  	fprintf(f, "\n  %s %d", name, ntohs(rta_getattr_u16(attr)));
> >  }
> >  
> > +static void flower_print_key_id(FILE *f, char *name,
> > +				struct rtattr *attr)
> 
> const char *name?
ack

> 
> 
> > +{
> > +	if (!attr)
> > +		return;
> > +	fprintf(f, "\n  %s %d", name, ntohl(rta_getattr_u32(attr)));
> > +}
> > +
> 
> Why short circuit, just change the order:
> 
> 	if (attr)
> 		fprintf(f, "\n  %s %s", name, ntohl(rta_getattr_u32(attr));
> 
> You might also want to introduce rta_getattr_be32()
ack

> 
> Please change, retest and resubmit both patches.
ack

Thanks for reviewing,
Amir
Amir Vadai Nov. 30, 2016, 7:42 a.m. UTC | #3
On Wed, Nov 30, 2016 at 9:17 AM, Amir Vadai <amir@vadai.me> wrote:
> On Tue, Nov 29, 2016 at 07:26:58PM -0800, Stephen Hemminger wrote:
(Sending again since I just discovered that Google Inbox is adding an
HTML part...)

>> The overall design is fine, just a couple nits with the code.
>>
>> > diff --git a/tc/f_flower.c b/tc/f_flower.c
>> > index 2d31d1aa832d..1cf0750b5b83 100644
>> > --- a/tc/f_flower.c
>> > +++ b/tc/f_flower.c
>>
>> >
>> > +static int flower_parse_key_id(char *str, int type, struct nlmsghdr *n)
>>
>> str is not modified, therefore use: const char *str
> ack
>
>>
>> > +{
>> > +   int ret;
>> > +   __be32 key_id;
>> > +
>> > +   ret = get_be32(&key_id, str, 10);
>> > +   if (ret)
>> > +           return -1;
>>
>> Traditionally netlink attributes are in host order, why was flower
>> chosen to be different?
> I don't know, maybe Jiri (cc'ed) can explain, but it is all over the
> flower code.
Now the right Jiri (Pirko) is CC'ed

>
>>
>> > +
>> > +   addattr32(n, MAX_MSG, type, key_id);
>> > +
>> > +   return 0;
>>
>>
>> Why lose the return value here?  Why not:
>>
>>       ret = get_be32(&key_id, str, 10);
>>       if (!ret)
>>               addattr32(n, MAX_MSG, type, key_id);
> The get_*() function can return only -1 or 0. But you are right, and it
> is better the way you suggested.  Changing accordingly in V3.
>
>>
>> > +}
>> > +
>> >  static int flower_parse_opt(struct filter_util *qu, char *handle,
>> >                         int argc, char **argv, struct nlmsghdr *n)
>> >  {
>> > @@ -339,6 +359,38 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
>> >                             fprintf(stderr, "Illegal \"src_port\"\n");
>> >                             return -1;
>> >                     }
>> > +           } else if (matches(*argv, "enc_dst_ip") == 0) {
>> > +                   NEXT_ARG();
>> > +                   ret = flower_parse_ip_addr(*argv, 0,
>> > +                                              TCA_FLOWER_KEY_ENC_IPV4_DST,
>> > +                                              TCA_FLOWER_KEY_ENC_IPV4_DST_MASK,
>> > +                                              TCA_FLOWER_KEY_ENC_IPV6_DST,
>> > +                                              TCA_FLOWER_KEY_ENC_IPV6_DST_MASK,
>> > +                                              n);
>> > +                   if (ret < 0) {
>> > +                           fprintf(stderr, "Illegal \"enc_dst_ip\"\n");
>> > +                           return -1;
>> > +                   }
>> > +           } else if (matches(*argv, "enc_src_ip") == 0) {
>> > +                   NEXT_ARG();
>> > +                   ret = flower_parse_ip_addr(*argv, 0,
>> > +                                              TCA_FLOWER_KEY_ENC_IPV4_SRC,
>> > +                                              TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK,
>> > +                                              TCA_FLOWER_KEY_ENC_IPV6_SRC,
>> > +                                              TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK,
>> > +                                              n);
>> > +                   if (ret < 0) {
>> > +                           fprintf(stderr, "Illegal \"enc_src_ip\"\n");
>> > +                           return -1;
>> > +                   }
>> > +           } else if (matches(*argv, "enc_key_id") == 0) {
>> > +                   NEXT_ARG();
>> > +                   ret = flower_parse_key_id(*argv,
>> > +                                             TCA_FLOWER_KEY_ENC_KEY_ID, n);
>> > +                   if (ret < 0) {
>> > +                           fprintf(stderr, "Illegal \"enc_key_id\"\n");
>> > +                           return -1;
>> > +                   }
>> >             } else if (matches(*argv, "action") == 0) {
>> >                     NEXT_ARG();
>> >                     ret = parse_action(&argc, &argv, TCA_FLOWER_ACT, n);
>> > @@ -509,6 +561,14 @@ static void flower_print_port(FILE *f, char *name, __u8 ip_proto,
>> >     fprintf(f, "\n  %s %d", name, ntohs(rta_getattr_u16(attr)));
>> >  }
>> >
>> > +static void flower_print_key_id(FILE *f, char *name,
>> > +                           struct rtattr *attr)
>>
>> const char *name?
> ack
>
>>
>>
>> > +{
>> > +   if (!attr)
>> > +           return;
>> > +   fprintf(f, "\n  %s %d", name, ntohl(rta_getattr_u32(attr)));
>> > +}
>> > +
>>
>> Why short circuit, just change the order:
>>
>>       if (attr)
>>               fprintf(f, "\n  %s %s", name, ntohl(rta_getattr_u32(attr));
>>
>> You might also want to introduce rta_getattr_be32()
> ack
>
>>
>> Please change, retest and resubmit both patches.
> ack
>
> Thanks for reviewing,
> Amir
Jiri Pirko Nov. 30, 2016, 8:13 a.m. UTC | #4
Wed, Nov 30, 2016 at 08:38:24AM CET, amirva@gmail.com wrote:
>On Wed, Nov 30, 2016 at 9:17 AM Amir Vadai <amir@vadai.me> wrote:
>
>> On Tue, Nov 29, 2016 at 07:26:58PM -0800, Stephen Hemminger wrote:
>> > The overall design is fine, just a couple nits with the code.
>> >
>> > > diff --git a/tc/f_flower.c b/tc/f_flower.c
>> > > index 2d31d1aa832d..1cf0750b5b83 100644
>> > > --- a/tc/f_flower.c
>> > > +++ b/tc/f_flower.c
>> >
>> > >
>> > > +static int flower_parse_key_id(char *str, int type, struct nlmsghdr
>> *n)
>> >
>> > str is not modified, therefore use: const char *str
>> ack
>>
>> >
>> > > +{
>> > > +   int ret;
>> > > +   __be32 key_id;
>> > > +
>> > > +   ret = get_be32(&key_id, str, 10);
>> > > +   if (ret)
>> > > +           return -1;
>> >
>> > Traditionally netlink attributes are in host order, why was flower
>> > chosen to be different?
>> I don't know, maybe Jiri (cc'ed) can explain, but it is all over the
>> flower code.
>>
>Now the right Jiri (Pirko) is CC'ed

There is a bunch of helpers inside the cls_flower.c to put and set
values, they work with generic char arrays of len.


>
>
>> >
>> > > +
>> > > +   addattr32(n, MAX_MSG, type, key_id);
>> > > +
>> > > +   return 0;
>> >
>> >
>> > Why lose the return value here?  Why not:
>> >
>> >       ret = get_be32(&key_id, str, 10);
>> >       if (!ret)
>> >               addattr32(n, MAX_MSG, type, key_id);
>> The get_*() function can return only -1 or 0. But you are right, and it
>> is better the way you suggested.  Changing accordingly in V3.
>>
>> >
>> > > +}
>> > > +
>> > >  static int flower_parse_opt(struct filter_util *qu, char *handle,
>> > >                         int argc, char **argv, struct nlmsghdr *n)
>> > >  {
>> > > @@ -339,6 +359,38 @@ static int flower_parse_opt(struct filter_util
>> *qu, char *handle,
>> > >                             fprintf(stderr, "Illegal \"src_port\"\n");
>> > >                             return -1;
>> > >                     }
>> > > +           } else if (matches(*argv, "enc_dst_ip") == 0) {
>> > > +                   NEXT_ARG();
>> > > +                   ret = flower_parse_ip_addr(*argv, 0,
>> > > +
>> TCA_FLOWER_KEY_ENC_IPV4_DST,
>> > > +
>> TCA_FLOWER_KEY_ENC_IPV4_DST_MASK,
>> > > +
>> TCA_FLOWER_KEY_ENC_IPV6_DST,
>> > > +
>> TCA_FLOWER_KEY_ENC_IPV6_DST_MASK,
>> > > +                                              n);
>> > > +                   if (ret < 0) {
>> > > +                           fprintf(stderr, "Illegal
>> \"enc_dst_ip\"\n");
>> > > +                           return -1;
>> > > +                   }
>> > > +           } else if (matches(*argv, "enc_src_ip") == 0) {
>> > > +                   NEXT_ARG();
>> > > +                   ret = flower_parse_ip_addr(*argv, 0,
>> > > +
>> TCA_FLOWER_KEY_ENC_IPV4_SRC,
>> > > +
>> TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK,
>> > > +
>> TCA_FLOWER_KEY_ENC_IPV6_SRC,
>> > > +
>> TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK,
>> > > +                                              n);
>> > > +                   if (ret < 0) {
>> > > +                           fprintf(stderr, "Illegal
>> \"enc_src_ip\"\n");
>> > > +                           return -1;
>> > > +                   }
>> > > +           } else if (matches(*argv, "enc_key_id") == 0) {
>> > > +                   NEXT_ARG();
>> > > +                   ret = flower_parse_key_id(*argv,
>> > > +
>>  TCA_FLOWER_KEY_ENC_KEY_ID, n);
>> > > +                   if (ret < 0) {
>> > > +                           fprintf(stderr, "Illegal
>> \"enc_key_id\"\n");
>> > > +                           return -1;
>> > > +                   }
>> > >             } else if (matches(*argv, "action") == 0) {
>> > >                     NEXT_ARG();
>> > >                     ret = parse_action(&argc, &argv, TCA_FLOWER_ACT,
>> n);
>> > > @@ -509,6 +561,14 @@ static void flower_print_port(FILE *f, char
>> *name, __u8 ip_proto,
>> > >     fprintf(f, "\n  %s %d", name, ntohs(rta_getattr_u16(attr)));
>> > >  }
>> > >
>> > > +static void flower_print_key_id(FILE *f, char *name,
>> > > +                           struct rtattr *attr)
>> >
>> > const char *name?
>> ack
>>
>> >
>> >
>> > > +{
>> > > +   if (!attr)
>> > > +           return;
>> > > +   fprintf(f, "\n  %s %d", name, ntohl(rta_getattr_u32(attr)));
>> > > +}
>> > > +
>> >
>> > Why short circuit, just change the order:
>> >
>> >       if (attr)
>> >               fprintf(f, "\n  %s %s", name, ntohl(rta_getattr_u32(attr));
>> >
>> > You might also want to introduce rta_getattr_be32()
>> ack
>>
>> >
>> > Please change, retest and resubmit both patches.
>> ack
>>
>> Thanks for reviewing,
>> Amir
>>
diff mbox

Patch

diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 74f76647753b..0e0b0cf4bb72 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -36,7 +36,11 @@  flower \- flow based traffic control filter
 .BR dst_ip " | " src_ip " } { "
 .IR ipv4_address " | " ipv6_address " } | { "
 .BR dst_port " | " src_port " } "
-.IR port_number " }"
+.IR port_number " } | "
+.B enc_key_id
+.IR KEY-ID " | {"
+.BR enc_dst_ip " | " enc_src_ip " } { "
+.IR ipv4_address " | " ipv6_address " } | "
 .SH DESCRIPTION
 The
 .B flower
@@ -121,6 +125,17 @@  which has to be specified in beforehand.
 Match on layer 4 protocol source or destination port number. Only available for
 .BR ip_proto " values " udp " and " tcp ,
 which has to be specified in beforehand.
+.TP
+.BI enc_key_id " NUMBER"
+.TQ
+.BI enc_dst_ip " ADDRESS"
+.TQ
+.BI enc_src_ip " ADDRESS"
+Match on IP tunnel metadata. Key id
+.I NUMBER
+is a 32 bit tunnel key id (e.g. VNI for VXLAN tunnel).
+.I ADDRESS
+must be a valid IPv4 or IPv6 address.
 .SH NOTES
 As stated above where applicable, matches of a certain layer implicitly depend
 on the matches of the next lower layer. Precisely, layer one and two matches (
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 2d31d1aa832d..1cf0750b5b83 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -41,7 +41,10 @@  static void explain(void)
 	fprintf(stderr, "                       dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n");
 	fprintf(stderr, "                       src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n");
 	fprintf(stderr, "                       dst_port PORT-NUMBER |\n");
-	fprintf(stderr, "                       src_port PORT-NUMBER }\n");
+	fprintf(stderr, "                       src_port PORT-NUMBER |\n");
+	fprintf(stderr, "                       enc_dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n");
+	fprintf(stderr, "                       enc_src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n");
+	fprintf(stderr, "                       enc_key_id [ KEY-ID ] }\n");
 	fprintf(stderr,	"       FILTERID := X:Y:Z\n");
 	fprintf(stderr,	"       ACTION-SPEC := ... look at individual actions\n");
 	fprintf(stderr,	"\n");
@@ -121,8 +124,9 @@  static int flower_parse_ip_addr(char *str, __be16 eth_type,
 		family = AF_INET;
 	} else if (eth_type == htons(ETH_P_IPV6)) {
 		family = AF_INET6;
+	} else if (!eth_type) {
+		family = AF_UNSPEC;
 	} else {
-		fprintf(stderr, "Illegal \"eth_type\" for ip address\n");
 		return -1;
 	}
 
@@ -130,8 +134,10 @@  static int flower_parse_ip_addr(char *str, __be16 eth_type,
 	if (ret)
 		return -1;
 
-	if (addr.family != family)
+	if (family && (addr.family != family)) {
+		fprintf(stderr, "Illegal \"eth_type\" for ip address\n");
 		return -1;
+	}
 
 	addattr_l(n, MAX_MSG, addr.family == AF_INET ? addr4_type : addr6_type,
 		  addr.data, addr.bytelen);
@@ -181,6 +187,20 @@  static int flower_parse_port(char *str, __u8 ip_port,
 	return 0;
 }
 
+static int flower_parse_key_id(char *str, int type, struct nlmsghdr *n)
+{
+	int ret;
+	__be32 key_id;
+
+	ret = get_be32(&key_id, str, 10);
+	if (ret)
+		return -1;
+
+	addattr32(n, MAX_MSG, type, key_id);
+
+	return 0;
+}
+
 static int flower_parse_opt(struct filter_util *qu, char *handle,
 			    int argc, char **argv, struct nlmsghdr *n)
 {
@@ -339,6 +359,38 @@  static int flower_parse_opt(struct filter_util *qu, char *handle,
 				fprintf(stderr, "Illegal \"src_port\"\n");
 				return -1;
 			}
+		} else if (matches(*argv, "enc_dst_ip") == 0) {
+			NEXT_ARG();
+			ret = flower_parse_ip_addr(*argv, 0,
+						   TCA_FLOWER_KEY_ENC_IPV4_DST,
+						   TCA_FLOWER_KEY_ENC_IPV4_DST_MASK,
+						   TCA_FLOWER_KEY_ENC_IPV6_DST,
+						   TCA_FLOWER_KEY_ENC_IPV6_DST_MASK,
+						   n);
+			if (ret < 0) {
+				fprintf(stderr, "Illegal \"enc_dst_ip\"\n");
+				return -1;
+			}
+		} else if (matches(*argv, "enc_src_ip") == 0) {
+			NEXT_ARG();
+			ret = flower_parse_ip_addr(*argv, 0,
+						   TCA_FLOWER_KEY_ENC_IPV4_SRC,
+						   TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK,
+						   TCA_FLOWER_KEY_ENC_IPV6_SRC,
+						   TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK,
+						   n);
+			if (ret < 0) {
+				fprintf(stderr, "Illegal \"enc_src_ip\"\n");
+				return -1;
+			}
+		} else if (matches(*argv, "enc_key_id") == 0) {
+			NEXT_ARG();
+			ret = flower_parse_key_id(*argv,
+						  TCA_FLOWER_KEY_ENC_KEY_ID, n);
+			if (ret < 0) {
+				fprintf(stderr, "Illegal \"enc_key_id\"\n");
+				return -1;
+			}
 		} else if (matches(*argv, "action") == 0) {
 			NEXT_ARG();
 			ret = parse_action(&argc, &argv, TCA_FLOWER_ACT, n);
@@ -509,6 +561,14 @@  static void flower_print_port(FILE *f, char *name, __u8 ip_proto,
 	fprintf(f, "\n  %s %d", name, ntohs(rta_getattr_u16(attr)));
 }
 
+static void flower_print_key_id(FILE *f, char *name,
+				struct rtattr *attr)
+{
+	if (!attr)
+		return;
+	fprintf(f, "\n  %s %d", name, ntohl(rta_getattr_u32(attr)));
+}
+
 static int flower_print_opt(struct filter_util *qu, FILE *f,
 			    struct rtattr *opt, __u32 handle)
 {
@@ -577,6 +637,25 @@  static int flower_print_opt(struct filter_util *qu, FILE *f,
 			  tb[TCA_FLOWER_KEY_TCP_SRC],
 			  tb[TCA_FLOWER_KEY_UDP_SRC]);
 
+	flower_print_ip_addr(f, "enc_dst_ip",
+			     tb[TCA_FLOWER_KEY_ENC_IPV4_DST_MASK] ?
+			     htons(ETH_P_IP) : htons(ETH_P_IPV6),
+			     tb[TCA_FLOWER_KEY_ENC_IPV4_DST],
+			     tb[TCA_FLOWER_KEY_ENC_IPV4_DST_MASK],
+			     tb[TCA_FLOWER_KEY_ENC_IPV6_DST],
+			     tb[TCA_FLOWER_KEY_ENC_IPV6_DST_MASK]);
+
+	flower_print_ip_addr(f, "enc_src_ip",
+			     tb[TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK] ?
+			     htons(ETH_P_IP) : htons(ETH_P_IPV6),
+			     tb[TCA_FLOWER_KEY_ENC_IPV4_SRC],
+			     tb[TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK],
+			     tb[TCA_FLOWER_KEY_ENC_IPV6_SRC],
+			     tb[TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK]);
+
+	flower_print_key_id(f, "enc_key_id",
+			    tb[TCA_FLOWER_KEY_ENC_KEY_ID]);
+
 	if (tb[TCA_FLOWER_FLAGS])  {
 		__u32 flags = rta_getattr_u32(tb[TCA_FLOWER_FLAGS]);