From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.4 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 274CFC2D0CD for ; Tue, 17 Dec 2019 16:30:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C973D21835 for ; Tue, 17 Dec 2019 16:30:35 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=cumulusnetworks.com header.i=@cumulusnetworks.com header.b="HT6uRgsi" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728569AbfLQQae (ORCPT ); Tue, 17 Dec 2019 11:30:34 -0500 Received: from mail-lj1-f194.google.com ([209.85.208.194]:45727 "EHLO mail-lj1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728250AbfLQQae (ORCPT ); Tue, 17 Dec 2019 11:30:34 -0500 Received: by mail-lj1-f194.google.com with SMTP id j26so2525754ljc.12 for ; Tue, 17 Dec 2019 08:30:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cumulusnetworks.com; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=Hxtg+YPeEh/CJ4PM9f7LQ6pCe3SqbPbTeTT9Ww5LcjE=; b=HT6uRgsiNb/eThMajw5UupWJSOZmSdm5zNrkj23Tsf6mkEvs0+V91gmlDj0tsNb/5b O4wlb6h6ufbaft78jUOoCukes8g83GHtwpw/JQTNliicZDVkG1mBq4XU5j31J9d9YZMF ARscxprCG2s1mQXQA1tCmxQOxXcsJBpT4NWRU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Hxtg+YPeEh/CJ4PM9f7LQ6pCe3SqbPbTeTT9Ww5LcjE=; b=SPF7FSm5Y+Y6YzhQR7MIfHApUspCFuFV/15PSLVtFzoFrn5uFHq4vaSnlo7mz9936K tcr8CvPpDxSt/16krmRueU6Hjcl2TIclHVDp5LXrj0L3Vx1lw/Gh/BcG9yCaots9/FQC wWPO1zQ+SRWxaNh0W3JgE7MhsFO+XenL1kEwHFq7/sxSKX/Mi8fKm5ilGSQWvkOFWRss i43KxrSsw9GYIHyKgvnIwmYhmf52m6d3eqTlQLKrZ9HrfRBamaE3pMUhoO2TENZfyfMP d05fI0sAKkpaeSGXW3PWWXdIAJe2VcSKs+etHv2fJp08XXqSnmBbftI3AKwZqdNrQ9dP kMFg== X-Gm-Message-State: APjAAAX1uUnlFn4TOWkZjUb7AXpVjDlbXmYi7e9iZ2TgtNdT+g2N8W4i bhJCid+y3OXlIrouHXrwjKy87A== X-Google-Smtp-Source: APXvYqxUjwrZS0rHP0TdP+4SpIDpWvHE3HfzPqlbo+em+QsQDIE+wfhwdoLWbJXBYrutq/V/OuY4MQ== X-Received: by 2002:a2e:9886:: with SMTP id b6mr3880532ljj.47.1576600231473; Tue, 17 Dec 2019 08:30:31 -0800 (PST) Received: from [192.168.0.107] (84-238-136-197.ip.btc-net.bg. [84.238.136.197]) by smtp.gmail.com with ESMTPSA id y29sm12866981ljd.88.2019.12.17.08.30.29 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 17 Dec 2019 08:30:30 -0800 (PST) Subject: Re: [PATCH net-next v2] openvswitch: add TTL decrement action To: Matteo Croce , netdev@vger.kernel.org, dev@openvswitch.org Cc: linux-kernel@vger.kernel.org, Pravin B Shelar , "David S. Miller" , Bindiya Kurle , Simon Horman , Ben Pfaff References: <20191217155102.46039-1-mcroce@redhat.com> From: Nikolay Aleksandrov Message-ID: Date: Tue, 17 Dec 2019 18:30:29 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1 MIME-Version: 1.0 In-Reply-To: <20191217155102.46039-1-mcroce@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 17/12/2019 17:51, 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,1 > in_port(1),eth(),eth_type(0x0800), packets:0, bytes:0, used:never, actions:dec_ttl{ttl<=1 action:(drop)},1,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-authored-by: Bindiya Kurle > Signed-off-by: Bindiya Kurle > Signed-off-by: Matteo Croce > --- > include/uapi/linux/openvswitch.h | 22 +++++++ > net/openvswitch/actions.c | 71 +++++++++++++++++++++ > net/openvswitch/flow_netlink.c | 105 +++++++++++++++++++++++++++++++ > 3 files changed, 198 insertions(+) > Hi Matteo, [snip] > +} > + > /* When 'last' is true, sample() should always consume the 'skb'. > * Otherwise, sample() should keep 'skb' intact regardless what > * actions are executed within sample(). > @@ -1176,6 +1201,44 @@ 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 = ipv6_hdr(skb); > + > + err = skb_ensure_writable(skb, skb_network_offset(skb) + > + sizeof(*nh)); skb_ensure_writable() calls pskb_may_pull() which may reallocate so nh might become invalid. It seems the IPv4 version below is ok as the ptr is reloaded. One q as I don't know ovs that much - can this action be called only with skb->protocol == ETH_P_IP/IPV6 ? I.e. Are we sure that if it's not v6, then it must be v4 ? Thanks, Nik > + if (unlikely(err)) > + return err; > + > + if (nh->hop_limit <= 1) > + return -EHOSTUNREACH; > + > + key->ip.ttl = --nh->hop_limit; > + } else { > + struct iphdr *nh = ip_hdr(skb); > + 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, struct sk_buff *skb, > struct sw_flow_key *key, > @@ -1347,6 +1410,14 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb, > > break; > } > + > + case OVS_ACTION_ATTR_DEC_TTL: > + err = execute_dec_ttl(skb, key); > + if (err == -EHOSTUNREACH) { > + err = dec_ttl(dp, skb, key, a, true); > + return err; > + } > + break; > } > > if (unlikely(err)) { > diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c > index 65c2e3458ff5..a9eea2ffb8b0 100644 > --- a/net/openvswitch/flow_netlink.c > +++ b/net/openvswitch/flow_netlink.c > @@ -61,6 +61,7 @@ static bool actions_may_change_flow(const struct nlattr *actions) > case OVS_ACTION_ATTR_RECIRC: > case OVS_ACTION_ATTR_TRUNC: > case OVS_ACTION_ATTR_USERSPACE: > + case OVS_ACTION_ATTR_DEC_TTL: > break; > > case OVS_ACTION_ATTR_CT: > @@ -2494,6 +2495,59 @@ static int validate_and_copy_sample(struct net *net, const struct nlattr *attr, > return 0; > } > > +static int validate_and_copy_dec_ttl(struct net *net, const struct nlattr *attr, > + const struct sw_flow_key *key, > + struct sw_flow_actions **sfa, > + __be16 eth_type, __be16 vlan_tci, > + u32 mpls_label_count, bool log) > +{ > + struct nlattr *attrs[OVS_DEC_TTL_ATTR_MAX + 1] = { 0 }; > + const struct nlattr *action_type, *action; > + struct nlattr *a; > + int rem, start, err; > + struct dec_ttl_arg arg; > + > + nla_for_each_nested(a, attr, rem) { > + int type = nla_type(a); > + > + if (!type || type > OVS_DEC_TTL_ATTR_MAX || attrs[type]) > + return -EINVAL; > + > + attrs[type] = a; > + } > + if (rem) > + return -EINVAL; > + > + action_type = attrs[OVS_DEC_TTL_ATTR_ACTION_TYPE]; > + if (!action_type || nla_len(action_type) != sizeof(u32)) > + return -EINVAL; > + > + start = add_nested_action_start(sfa, OVS_ACTION_ATTR_DEC_TTL, log); > + if (start < 0) > + return start; > + > + arg.action_type = nla_get_u32(action_type); > + err = ovs_nla_add_action(sfa, OVS_DEC_TTL_ATTR_ARG, > + &arg, sizeof(arg), log); > + if (err) > + return err; > + > + if (arg.action_type == OVS_DEC_TTL_ACTION_USER_SPACE) { > + action = attrs[OVS_DEC_TTL_ATTR_ACTION]; > + if (!action || (nla_len(action) && nla_len(action) < NLA_HDRLEN)) > + return -EINVAL; > + > + err = __ovs_nla_copy_actions(net, action, key, sfa, eth_type, > + vlan_tci, mpls_label_count, log); > + if (err) > + return err; > + } > + > + add_nested_action_end(*sfa, start); > + > + return 0; > +} > + > static int validate_and_copy_clone(struct net *net, > const struct nlattr *attr, > const struct sw_flow_key *key, > @@ -3005,6 +3059,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, > [OVS_ACTION_ATTR_METER] = sizeof(u32), > [OVS_ACTION_ATTR_CLONE] = (u32)-1, > [OVS_ACTION_ATTR_CHECK_PKT_LEN] = (u32)-1, > + [OVS_ACTION_ATTR_DEC_TTL] = (u32)-1, > }; > const struct ovs_action_push_vlan *vlan; > int type = nla_type(a); > @@ -3233,6 +3288,15 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, > break; > } > > + case OVS_ACTION_ATTR_DEC_TTL: > + err = validate_and_copy_dec_ttl(net, a, key, sfa, > + eth_type, vlan_tci, > + mpls_label_count, log); > + if (err) > + return err; > + skip_copy = true; > + break; > + > default: > OVS_NLERR(log, "Unknown Action type %d", type); > return -EINVAL; > @@ -3404,6 +3468,41 @@ static int check_pkt_len_action_to_attr(const struct nlattr *attr, > return err; > } > > +static int dec_ttl_action_to_attr(const struct nlattr *att, struct sk_buff *skb) > +{ > + struct nlattr *start, *ac_start = NULL, *dec_ttl; > + int err = 0, rem = nla_len(att); > + const struct dec_ttl_arg *arg; > + struct nlattr *actions; > + > + start = nla_nest_start_noflag(skb, OVS_ACTION_ATTR_DEC_TTL); > + if (!start) > + return -EMSGSIZE; > + > + dec_ttl = nla_data(att); > + arg = nla_data(dec_ttl); > + actions = nla_next(dec_ttl, &rem); > + > + if (nla_put_u32(skb, OVS_DEC_TTL_ATTR_ACTION_TYPE, arg->action_type)) { > + nla_nest_cancel(skb, start); > + return -EMSGSIZE; > + } > + > + if (arg->action_type == OVS_DEC_TTL_ACTION_USER_SPACE) { > + ac_start = nla_nest_start_noflag(skb, OVS_DEC_TTL_ATTR_ACTION); > + if (!ac_start) { > + nla_nest_cancel(skb, ac_start); > + nla_nest_cancel(skb, start); > + return -EMSGSIZE; > + } > + err = ovs_nla_put_actions(actions, rem, skb); > + nla_nest_end(skb, ac_start); > + } > + nla_nest_end(skb, start); > + > + return err; > +} > + > static int set_action_to_attr(const struct nlattr *a, struct sk_buff *skb) > { > const struct nlattr *ovs_key = nla_data(a); > @@ -3504,6 +3603,12 @@ int ovs_nla_put_actions(const struct nlattr *attr, int len, struct sk_buff *skb) > return err; > break; > > + case OVS_ACTION_ATTR_DEC_TTL: > + err = dec_ttl_action_to_attr(a, skb); > + if (err) > + return err; > + break; > + > default: > if (nla_put(skb, type, nla_len(a), nla_data(a))) > return -EMSGSIZE; >