Re: [ovs-dev] [PATCH net-next v3] openvswitch: add TTL decrement action
Le 16/01/2020 à 08:21, Pravin Shelar a écrit :
> On Wed, Jan 15, 2020 at 8:40 AM Matteo Croce wrote:
[snip]
>> @@ -1050,4 +1051,5 @@ struct ovs_zone_limit {
>> __u32 count;
>> };
>>
>> +#define OVS_DEC_TTL_ATTR_EXEC 0
>
> I am not sure if we need this, But if you want the nested attribute
> then lets define enum with this single attribute and have actions as
> part of its data. This would be optional argument, so userspace can
> skip it, and in that case datapath can drop the packet.
And note that 0 is a reserved value and should not be used. Look at other
attributes.
Regards,
Nicolas
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v3] openvswitch: add TTL decrement action
On Wed, Jan 15, 2020 at 8:40 AM Matteo Croce wrote:
>
> New action to decrement TTL instead of setting it to a fixed value.
> This action will decrement the TTL and, in case of expired TTL, drop it
> or execute an action passed via a nested attribute.
> The default TTL expired action is to drop the packet.
>
> Supports both IPv4 and IPv6 via the ttl and hop_limit fields, respectively.
>
> Tested with a corresponding change in the userspace:
>
> # ovs-dpctl dump-flows
> in_port(2),eth(),eth_type(0x0800), packets:0, bytes:0, used:never,
> actions:dec_ttl{ttl<=1 action:(drop)},1
> in_port(1),eth(),eth_type(0x0800), packets:0, bytes:0, used:never,
> actions:dec_ttl{ttl<=1 action:(drop)},2
> in_port(1),eth(),eth_type(0x0806), packets:0, bytes:0, used:never,
> actions:2
> in_port(2),eth(),eth_type(0x0806), packets:0, bytes:0, used:never,
> actions:1
>
> # ping -c1 192.168.0.2 -t 42
> IP (tos 0x0, ttl 41, id 61647, offset 0, flags [DF], proto ICMP (1),
> length 84)
> 192.168.0.1 > 192.168.0.2: ICMP echo request, id 386, seq 1, length 64
> # ping -c1 192.168.0.2 -t 120
> IP (tos 0x0, ttl 119, id 62070, offset 0, flags [DF], proto ICMP (1),
> length 84)
> 192.168.0.1 > 192.168.0.2: ICMP echo request, id 388, seq 1, length 64
> # ping -c1 192.168.0.2 -t 1
> #
>
Thanks for the patch.
> Co-developed-by: Bindiya Kurle
> Signed-off-by: Bindiya Kurle
> Signed-off-by: Matteo Croce
> ---
> include/uapi/linux/openvswitch.h | 2 +
> net/openvswitch/actions.c| 67 ++
> net/openvswitch/flow_netlink.c | 71
> 3 files changed, 140 insertions(+)
>
> diff --git a/include/uapi/linux/openvswitch.h
> b/include/uapi/linux/openvswitch.h
> index ae2bff14e7e1..9d3f040847af 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -958,6 +958,7 @@ enum ovs_action_attr {
> OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*. */
> OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
> OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
> + OVS_ACTION_ATTR_DEC_TTL, /* Nested OVS_DEC_TTL_ATTR_*. */
>
> __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
>* from userspace. */
> @@ -1050,4 +1051,5 @@ struct ovs_zone_limit {
> __u32 count;
> };
>
> +#define OVS_DEC_TTL_ATTR_EXEC 0
I am not sure if we need this, But if you want the nested attribute
then lets define enum with this single attribute and have actions as
part of its data. This would be optional argument, so userspace can
skip it, and in that case datapath can drop the packet.
> #endif /* _LINUX_OPENVSWITCH_H */
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 7fbfe2adfffa..1b0afc9bf1ad 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -964,6 +964,26 @@ static int output_userspace(struct datapath *dp, struct
> sk_buff *skb,
> return ovs_dp_upcall(dp, skb, key, &upcall, cutlen);
> }
>
> +static int dec_ttl(struct datapath *dp, struct sk_buff *skb,
> + struct sw_flow_key *key,
> + const struct nlattr *attr, bool last)
Can you give it better name, for example: dec_ttl_exception_handler().
> +{
> + /* The first action is always 'OVS_DEC_TTL_ATTR_ARG'. */
> + struct nlattr *dec_ttl_arg = nla_data(attr);
> + u32 nested = nla_get_u32(dec_ttl_arg);
> + int rem = nla_len(attr);
> +
> + if (nested) {
> + struct nlattr *actions = nla_next(dec_ttl_arg, &rem);
> +
> + if (actions)
> + return clone_execute(dp, skb, key, 0, actions, rem,
> +last, false);
> + }
> + consume_skb(skb);
> + return 0;
> +}
> +
> /* When 'last' is true, sample() should always consume the 'skb'.
> * Otherwise, sample() should keep 'skb' intact regardless what
> * actions are executed within sample().
> @@ -1180,6 +1200,45 @@ static int execute_check_pkt_len(struct datapath *dp,
> struct sk_buff *skb,
> nla_len(actions), last, clone_flow_key);
> }
>
> +static int execute_dec_ttl(struct sk_buff *skb, struct sw_flow_key *key)
> +{
> + int err;
> +
> + if (skb->protocol == htons(ETH_P_IPV6)) {
> + struct ipv6hdr *nh;
> +
> + err = skb_ensure_writable(skb, skb_network_offset(skb) +
> + sizeof(*nh));
> + if (unlikely(err))
> + return err;
> +
> + nh = ipv6_hdr(skb);
> +
> + if (nh->hop_limit <= 1)
> + return -EHOSTUNREACH;
> +
> + key->ip.ttl = --nh->hop_limit;
> + } else {
> + struct iphdr *nh;
> + u8 old_ttl;
> +
> +
[ovs-dev] [PATCH net-next v3] openvswitch: add TTL decrement action
New action to decrement TTL instead of setting it to a fixed value.
This action will decrement the TTL and, in case of expired TTL, drop it
or execute an action passed via a nested attribute.
The default TTL expired action is to drop the packet.
Supports both IPv4 and IPv6 via the ttl and hop_limit fields, respectively.
Tested with a corresponding change in the userspace:
# ovs-dpctl dump-flows
in_port(2),eth(),eth_type(0x0800), packets:0, bytes:0, used:never,
actions:dec_ttl{ttl<=1 action:(drop)},1
in_port(1),eth(),eth_type(0x0800), packets:0, bytes:0, used:never,
actions:dec_ttl{ttl<=1 action:(drop)},2
in_port(1),eth(),eth_type(0x0806), packets:0, bytes:0, used:never, actions:2
in_port(2),eth(),eth_type(0x0806), packets:0, bytes:0, used:never, actions:1
# ping -c1 192.168.0.2 -t 42
IP (tos 0x0, ttl 41, id 61647, offset 0, flags [DF], proto ICMP (1), length
84)
192.168.0.1 > 192.168.0.2: ICMP echo request, id 386, seq 1, length 64
# ping -c1 192.168.0.2 -t 120
IP (tos 0x0, ttl 119, id 62070, offset 0, flags [DF], proto ICMP (1),
length 84)
192.168.0.1 > 192.168.0.2: ICMP echo request, id 388, seq 1, length 64
# ping -c1 192.168.0.2 -t 1
#
Co-developed-by: Bindiya Kurle
Signed-off-by: Bindiya Kurle
Signed-off-by: Matteo Croce
---
include/uapi/linux/openvswitch.h | 2 +
net/openvswitch/actions.c| 67 ++
net/openvswitch/flow_netlink.c | 71
3 files changed, 140 insertions(+)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index ae2bff14e7e1..9d3f040847af 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -958,6 +958,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*. */
OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
+ OVS_ACTION_ATTR_DEC_TTL, /* Nested OVS_DEC_TTL_ATTR_*. */
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
* from userspace. */
@@ -1050,4 +1051,5 @@ struct ovs_zone_limit {
__u32 count;
};
+#define OVS_DEC_TTL_ATTR_EXEC 0
#endif /* _LINUX_OPENVSWITCH_H */
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 7fbfe2adfffa..1b0afc9bf1ad 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -964,6 +964,26 @@ static int output_userspace(struct datapath *dp, struct
sk_buff *skb,
return ovs_dp_upcall(dp, skb, key, &upcall, cutlen);
}
+static int dec_ttl(struct datapath *dp, struct sk_buff *skb,
+ struct sw_flow_key *key,
+ const struct nlattr *attr, bool last)
+{
+ /* The first action is always 'OVS_DEC_TTL_ATTR_ARG'. */
+ struct nlattr *dec_ttl_arg = nla_data(attr);
+ u32 nested = nla_get_u32(dec_ttl_arg);
+ int rem = nla_len(attr);
+
+ if (nested) {
+ struct nlattr *actions = nla_next(dec_ttl_arg, &rem);
+
+ if (actions)
+ return clone_execute(dp, skb, key, 0, actions, rem,
+last, false);
+ }
+ consume_skb(skb);
+ return 0;
+}
+
/* When 'last' is true, sample() should always consume the 'skb'.
* Otherwise, sample() should keep 'skb' intact regardless what
* actions are executed within sample().
@@ -1180,6 +1200,45 @@ static int execute_check_pkt_len(struct datapath *dp,
struct sk_buff *skb,
nla_len(actions), last, clone_flow_key);
}
+static int execute_dec_ttl(struct sk_buff *skb, struct sw_flow_key *key)
+{
+ int err;
+
+ if (skb->protocol == htons(ETH_P_IPV6)) {
+ struct ipv6hdr *nh;
+
+ err = skb_ensure_writable(skb, skb_network_offset(skb) +
+ sizeof(*nh));
+ if (unlikely(err))
+ return err;
+
+ nh = ipv6_hdr(skb);
+
+ if (nh->hop_limit <= 1)
+ return -EHOSTUNREACH;
+
+ key->ip.ttl = --nh->hop_limit;
+ } else {
+ struct iphdr *nh;
+ u8 old_ttl;
+
+ err = skb_ensure_writable(skb, skb_network_offset(skb) +
+ sizeof(*nh));
+ if (unlikely(err))
+ return err;
+
+ nh = ip_hdr(skb);
+ if (nh->ttl <= 1)
+ return -EHOSTUNREACH;
+
+ old_ttl = nh->ttl--;
+ csum_replace2(&nh->check, htons(old_ttl << 8),
+ htons(nh->ttl << 8));
+ key->ip.ttl = nh->ttl;
+ }
+ return 0;
+}
+
/* Execute a list of actions against 'skb'. */
static int do_execute_actions(struct datapath *dp
