From patchwork Tue Jun 21 00:19:38 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jarno Rajahalme X-Patchwork-Id: 638386 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3rYW9R2YCcz9sdg for ; Tue, 21 Jun 2016 11:58:51 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753188AbcFUB6s (ORCPT ); Mon, 20 Jun 2016 21:58:48 -0400 Received: from slow1-d.mail.gandi.net ([217.70.178.86]:36714 "EHLO slow1-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751369AbcFUB6r (ORCPT ); Mon, 20 Jun 2016 21:58:47 -0400 Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by slow1-d.mail.gandi.net (Postfix) with ESMTP id BB6E347F87A for ; Tue, 21 Jun 2016 02:21:41 +0200 (CEST) Received: from mfilter28-d.gandi.net (mfilter28-d.gandi.net [217.70.178.159]) by relay4-d.mail.gandi.net (Postfix) with ESMTP id 9400117209C; Tue, 21 Jun 2016 02:20:29 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at mfilter28-d.gandi.net Received: from relay4-d.mail.gandi.net ([IPv6:::ffff:217.70.183.196]) by mfilter28-d.gandi.net (mfilter28-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id e_juii9anAVt; Tue, 21 Jun 2016 02:20:28 +0200 (CEST) X-Originating-IP: 208.91.1.34 Received: from sc9-mailhost2.vmware.com (unknown [208.91.1.34]) (Authenticated sender: jarno@ovn.org) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 2271B1720A3; Tue, 21 Jun 2016 02:20:26 +0200 (CEST) From: Jarno Rajahalme To: netdev@vger.kernel.org Cc: dev@openvswitch.org, jarno@ovn.org Subject: [PATCH net-next] openvswitch: Only set mark and labels when commiting a connection. Date: Mon, 20 Jun 2016 17:19:38 -0700 Message-Id: <1466468378-32235-1-git-send-email-jarno@ovn.org> X-Mailer: git-send-email 2.1.4 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Only allow setting conntrack mark or labels when the commit flag is specified. This makes sure we can not set them before the connection has been persisted, as in that case the mark and labels would be lost in an event of an userspace upcall. OVS userspace already requires the commit flag to accept setting ct_mark and/or ct_labels. Validate for this on the kernel API. Finally, set conntrack mark and labels right before committing so that the initial conntrack NEW event has the mark and labels. Signed-off-by: Jarno Rajahalme --- net/openvswitch/conntrack.c | 72 ++++++++++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 24 deletions(-) diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c index 3d5feed..f1612f5 100644 --- a/net/openvswitch/conntrack.c +++ b/net/openvswitch/conntrack.c @@ -824,6 +824,17 @@ static int ovs_ct_lookup(struct net *net, struct sw_flow_key *key, return 0; } +static bool labels_nonzero(const struct ovs_key_ct_labels *labels) +{ + size_t i; + + for (i = 0; i < sizeof(*labels); i++) + if (labels->ct_labels[i]) + return true; + + return false; +} + /* Lookup connection and confirm if unconfirmed. */ static int ovs_ct_commit(struct net *net, struct sw_flow_key *key, const struct ovs_conntrack_info *info, @@ -834,24 +845,35 @@ static int ovs_ct_commit(struct net *net, struct sw_flow_key *key, err = __ovs_ct_lookup(net, key, info, skb); if (err) return err; - /* This is a no-op if the connection has already been confirmed. */ + + /* As any changes to an unconfirmed connection may be lost due + * to an upcall, we require the presence of the 'commit' flag + * for setting mask and/or labels. Perform the changes before + * confirming the connection so that the initial mark and label + * values are present in the initial CT NEW netlink event. + */ + if (info->mark.mask) { + err = ovs_ct_set_mark(skb, key, info->mark.value, + info->mark.mask); + if (err) + return err; + } + if (labels_nonzero(&info->labels.mask)) { + err = ovs_ct_set_labels(skb, key, &info->labels.value, + &info->labels.mask); + if (err) + return err; + } + + /* Will take care of sending queued events even if the connection is + * already confirmed. + */ if (nf_conntrack_confirm(skb) != NF_ACCEPT) return -EINVAL; return 0; } -static bool labels_nonzero(const struct ovs_key_ct_labels *labels) -{ - size_t i; - - for (i = 0; i < sizeof(*labels); i++) - if (labels->ct_labels[i]) - return true; - - return false; -} - /* Returns 0 on success, -EINPROGRESS if 'skb' is stolen, or other nonzero * value if 'skb' is freed. */ @@ -876,19 +898,7 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb, err = ovs_ct_commit(net, key, info, skb); else err = ovs_ct_lookup(net, key, info, skb); - if (err) - goto err; - if (info->mark.mask) { - err = ovs_ct_set_mark(skb, key, info->mark.value, - info->mark.mask); - if (err) - goto err; - } - if (labels_nonzero(&info->labels.mask)) - err = ovs_ct_set_labels(skb, key, &info->labels.value, - &info->labels.mask); -err: skb_push(skb, nh_ofs); if (err) kfree_skb(skb); @@ -1145,6 +1155,20 @@ static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info, } } +#ifdef CONFIG_NF_CONNTRACK_MARK + if (!info->commit && info->mark.mask) { + OVS_NLERR(log, + "Setting conntrack mark requires 'commit' flag."); + return -EINVAL; + } +#endif +#ifdef CONFIG_NF_CONNTRACK_LABELS + if (!info->commit && labels_nonzero(&info->labels.mask)) { + OVS_NLERR(log, + "Setting conntrack labels requires 'commit' flag."); + return -EINVAL; + } +#endif if (rem > 0) { OVS_NLERR(log, "Conntrack attr has %d unknown bytes", rem); return -EINVAL;