Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753337AbbHMROG (ORCPT ); Thu, 13 Aug 2015 13:14:06 -0400 Received: from mail-io0-f169.google.com ([209.85.223.169]:36702 "EHLO mail-io0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752246AbbHMROD (ORCPT ); Thu, 13 Aug 2015 13:14:03 -0400 MIME-Version: 1.0 In-Reply-To: References: <1439333961-24474-1-git-send-email-joestringer@nicira.com> <1439333961-24474-7-git-send-email-joestringer@nicira.com> Date: Thu, 13 Aug 2015 10:14:02 -0700 Message-ID: Subject: Re: [PATCHv3 net-next 06/10] openvswitch: Allow matching on conntrack mark From: Pravin Shelar To: Joe Stringer Cc: netdev , Justin Pettit , LKML , pablo , Patrick McHardy , Andy Zhou , Jesse Gross , Florian Westphal , Hannes Sowa , Thomas Graf Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2540 Lines: 60 On Wed, Aug 12, 2015 at 4:41 PM, Joe Stringer wrote: > On 12 August 2015 at 16:00, Pravin Shelar wrote: >> On Tue, Aug 11, 2015 at 3:59 PM, Joe Stringer wrote: >>> From: Justin Pettit >>> >>> Allow matching and setting the conntrack mark field. As with conntrack >>> state and zone, these are populated by executing the ct() action. Unlike >>> these, the ct_mark is also a writable field. The set_field() action may >>> be used to modify the mark, which will take effect on the most recent >>> conntrack entry. >>> >>> E.g.: actions:ct(zone=0),ct(zone=1),set_field(1->ct_mark) >>> >>> This will perform conntrack lookup in zone 0, then lookup in zone 1, >>> then modify the mark for the entry in zone 1. The mark for the entry in >>> zone 0 is unchanged. The conntrack entry itself must be committed using >>> the "commit" flag in the conntrack action flags for this change to persist. >>> >>> Signed-off-by: Justin Pettit >>> Signed-off-by: Joe Stringer >>> --- >> ... >> >>> >>> +int ovs_ct_set_mark(struct sk_buff *skb, struct sw_flow_key *key, >>> + u32 ct_mark, u32 mask) >>> +{ >>> +#ifdef CONFIG_NF_CONNTRACK_MARK >>> + enum ip_conntrack_info ctinfo; >>> + struct nf_conn *ct; >>> + u32 new_mark; >>> + >>> + /* This must happen directly after lookup/commit. */ >>> + ct = nf_ct_get(skb, &ctinfo); >>> + if (!ct) >>> + return -EINVAL; >>> + >>> + new_mark = ct_mark | (ct->mark & ~(mask)); >>> + if (ct->mark != new_mark) { >>> + ct->mark = new_mark; >>> + nf_conntrack_event_cache(IPCT_MARK, ct); >>> + key->ct.mark = ct_mark; >>> + } >>> + >> >> Is it fine to set just set mark and not initialize reset of key->ct members? > > I don't quite follow. This action acts upon the current connection, > and modifies its metadata. key->ct should already be populated with > the existing connection info. I had offline discussion with Joe. The fields are initialized in prior conntrack action. So now he is exploring if we can bring conntrack, set mark and set lable actions under one single conntrack action using parameters. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/