diff mbox

[ovs-dev,2/2] datapath: Add eventmask support to CT action.

Message ID 1493067004-31730-2-git-send-email-jarno@ovn.org
State Accepted
Delegated to: Joe Stringer
Headers show

Commit Message

Jarno Rajahalme April 24, 2017, 8:50 p.m. UTC
Upstream commit:

    commit 120645513f55a4ac5543120d9e79925d30a0156f
    Author: Jarno Rajahalme <jarno@ovn.org>
    Date:   Fri Apr 21 16:48:06 2017 -0700

    openvswitch: Add eventmask support to CT action.

    Add a new optional conntrack action attribute OVS_CT_ATTR_EVENTMASK,
    which can be used in conjunction with the commit flag
    (OVS_CT_ATTR_COMMIT) to set the mask of bits specifying which
    conntrack events (IPCT_*) should be delivered via the Netfilter
    netlink multicast groups.  Default behavior depends on the system
    configuration, but typically a lot of events are delivered.  This can be
    very chatty for the NFNLGRP_CONNTRACK_UPDATE group, even if only some
    types of events are of interest.

    Netfilter core init_conntrack() adds the event cache extension, so we
    only need to set the ctmask value.  However, if the system is
    configured without support for events, the setting will be skipped due
    to extension not being found.

    Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
    Reviewed-by: Greg Rose <gvrose8192@gmail.com>
    Acked-by: Joe Stringer <joe@ovn.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
 datapath/conntrack.c                              | 27 +++++++++++++++++++++++
 datapath/linux/compat/include/linux/openvswitch.h | 12 ++++++++++
 lib/dpif-netdev.c                                 |  4 ++++
 3 files changed, 43 insertions(+)

Comments

Gregory Rose April 24, 2017, 9:03 p.m. UTC | #1
On Mon, 2017-04-24 at 13:50 -0700, Jarno Rajahalme wrote:
> Upstream commit:
> 
>     commit 120645513f55a4ac5543120d9e79925d30a0156f
>     Author: Jarno Rajahalme <jarno@ovn.org>
>     Date:   Fri Apr 21 16:48:06 2017 -0700
> 
>     openvswitch: Add eventmask support to CT action.
> 
>     Add a new optional conntrack action attribute OVS_CT_ATTR_EVENTMASK,
>     which can be used in conjunction with the commit flag
>     (OVS_CT_ATTR_COMMIT) to set the mask of bits specifying which
>     conntrack events (IPCT_*) should be delivered via the Netfilter
>     netlink multicast groups.  Default behavior depends on the system
>     configuration, but typically a lot of events are delivered.  This can be
>     very chatty for the NFNLGRP_CONNTRACK_UPDATE group, even if only some
>     types of events are of interest.
> 
>     Netfilter core init_conntrack() adds the event cache extension, so we
>     only need to set the ctmask value.  However, if the system is
>     configured without support for events, the setting will be skipped due
>     to extension not being found.
> 
>     Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>     Reviewed-by: Greg Rose <gvrose8192@gmail.com>
>     Acked-by: Joe Stringer <joe@ovn.org>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>

On patches that have already been reviewed, accepted by the community
and pushed upstream do we still ACK those here on the OVS dev list?

Just want to keep noise down if they're not considered necessary.

Thanks,

- Greg

> ---
>  datapath/conntrack.c                              | 27 +++++++++++++++++++++++
>  datapath/linux/compat/include/linux/openvswitch.h | 12 ++++++++++
>  lib/dpif-netdev.c                                 |  4 ++++
>  3 files changed, 43 insertions(+)
> 
> diff --git a/datapath/conntrack.c b/datapath/conntrack.c
> index f911fe8..95c3739 100644
> --- a/datapath/conntrack.c
> +++ b/datapath/conntrack.c
> @@ -73,7 +73,9 @@ struct ovs_conntrack_info {
>  	u8 nat : 3;                 /* enum ovs_ct_nat */
>  	u8 random_fully_compat : 1; /* bool */
>  	u8 force : 1;
> +	u8 have_eventmask : 1;
>  	u16 family;
> +	u32 eventmask;              /* Mask of 1 << IPCT_*. */
>  	struct md_mark mark;
>  	struct md_labels labels;
>  #ifdef CONFIG_NF_NAT_NEEDED
> @@ -1041,6 +1043,20 @@ static int ovs_ct_commit(struct net *net, struct sw_flow_key *key,
>  	if (!ct)
>  		return 0;
>  
> +	/* Set the conntrack event mask if given.  NEW and DELETE events have
> +	 * their own groups, but the NFNLGRP_CONNTRACK_UPDATE group listener
> +	 * typically would receive many kinds of updates.  Setting the event
> +	 * mask allows those events to be filtered.  The set event mask will
> +	 * remain in effect for the lifetime of the connection unless changed
> +	 * by a further CT action with both the commit flag and the eventmask
> +	 * option. */
> +	if (info->have_eventmask) {
> +		struct nf_conntrack_ecache *cache = nf_ct_ecache_find(ct);
> +
> +		if (cache)
> +			cache->ctmask = info->eventmask;
> +	}
> +
>  	/* Apply changes before confirming the connection so that the initial
>  	 * conntrack NEW netlink event carries the values given in the CT
>  	 * action.
> @@ -1277,6 +1293,8 @@ static const struct ovs_ct_len_tbl ovs_ct_attr_lens[OVS_CT_ATTR_MAX + 1] = {
>  	/* NAT length is checked when parsing the nested attributes. */
>  	[OVS_CT_ATTR_NAT]	= { .minlen = 0, .maxlen = INT_MAX },
>  #endif
> +	[OVS_CT_ATTR_EVENTMASK]	= { .minlen = sizeof(u32),
> +				    .maxlen = sizeof(u32) },
>  };
>  
>  static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
> @@ -1355,6 +1373,11 @@ static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
>  			break;
>  		}
>  #endif
> +		case OVS_CT_ATTR_EVENTMASK:
> +			info->have_eventmask = true;
> +			info->eventmask = nla_get_u32(a);
> +			break;
> +
>  		default:
>  			OVS_NLERR(log, "Unknown conntrack attr (%d)",
>  				  type);
> @@ -1558,6 +1581,10 @@ int ovs_ct_action_to_attr(const struct ovs_conntrack_info *ct_info,
>  				   ct_info->helper->name))
>  			return -EMSGSIZE;
>  	}
> +	if (ct_info->have_eventmask &&
> +	    nla_put_u32(skb, OVS_CT_ATTR_EVENTMASK, ct_info->eventmask))
> +		return -EMSGSIZE;
> +
>  #ifdef CONFIG_NF_NAT_NEEDED
>  	if (ct_info->nat && !ovs_ct_nat_to_attr(ct_info, skb))
>  		return -EMSGSIZE;
> diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
> index 8a6b729..72627f9 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -739,6 +739,17 @@ struct ovs_action_push_tnl {
>   * nothing if the connection is already committed will check that the current
>   * packet is in conntrack entry's original direction.  If directionality does
>   * not match, will delete the existing conntrack entry and create a new one.
> + * @OVS_CT_ATTR_EVENTMASK: Mask of bits indicating which conntrack event types
> + * (enum ip_conntrack_events IPCT_*) should be reported.  For any bit set to
> + * zero, the corresponding event type is not generated.  Default behavior
> + * depends on system configuration, but typically all event types are
> + * generated, hence listening on NFNLGRP_CONNTRACK_UPDATE events may get a lot
> + * of events.  Explicitly passing this attribute allows limiting the updates
> + * received to the events of interest.  The bit 1 << IPCT_NEW, 1 <<
> + * IPCT_RELATED, and 1 << IPCT_DESTROY must be set to ones for those events to
> + * be received on NFNLGRP_CONNTRACK_NEW and NFNLGRP_CONNTRACK_DESTROY groups,
> + * respectively.  Remaining bits control the changes for which an event is
> + * delivered on the NFNLGRP_CONNTRACK_UPDATE group.
>   */
>  enum ovs_ct_attr {
>  	OVS_CT_ATTR_UNSPEC,
> @@ -750,6 +761,7 @@ enum ovs_ct_attr {
>  				   related connections. */
>  	OVS_CT_ATTR_NAT,        /* Nested OVS_NAT_ATTR_* */
>  	OVS_CT_ATTR_FORCE_COMMIT,  /* No argument */
> +	OVS_CT_ATTR_EVENTMASK,  /* u32 mask of IPCT_* events. */
>  	__OVS_CT_ATTR_MAX
>  };
>  
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 41d0836..b3a0806 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -5136,6 +5136,10 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
>              case OVS_CT_ATTR_LABELS:
>                  setlabel = nl_attr_get(b);
>                  break;
> +            case OVS_CT_ATTR_EVENTMASK:
> +                /* Silently ignored, as userspace datapath does not generate
> +                 * netlink events. */
> +                break;
>              case OVS_CT_ATTR_NAT:
>              case OVS_CT_ATTR_UNSPEC:
>              case __OVS_CT_ATTR_MAX:
Joe Stringer April 24, 2017, 9:20 p.m. UTC | #2
On 24 April 2017 at 14:03, Greg Rose <gvrose8192@gmail.com> wrote:
> On Mon, 2017-04-24 at 13:50 -0700, Jarno Rajahalme wrote:
>> Upstream commit:
>>
>>     commit 120645513f55a4ac5543120d9e79925d30a0156f
>>     Author: Jarno Rajahalme <jarno@ovn.org>
>>     Date:   Fri Apr 21 16:48:06 2017 -0700
>>
>>     openvswitch: Add eventmask support to CT action.
>>
>>     Add a new optional conntrack action attribute OVS_CT_ATTR_EVENTMASK,
>>     which can be used in conjunction with the commit flag
>>     (OVS_CT_ATTR_COMMIT) to set the mask of bits specifying which
>>     conntrack events (IPCT_*) should be delivered via the Netfilter
>>     netlink multicast groups.  Default behavior depends on the system
>>     configuration, but typically a lot of events are delivered.  This can be
>>     very chatty for the NFNLGRP_CONNTRACK_UPDATE group, even if only some
>>     types of events are of interest.
>>
>>     Netfilter core init_conntrack() adds the event cache extension, so we
>>     only need to set the ctmask value.  However, if the system is
>>     configured without support for events, the setting will be skipped due
>>     to extension not being found.
>>
>>     Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>>     Reviewed-by: Greg Rose <gvrose8192@gmail.com>
>>     Acked-by: Joe Stringer <joe@ovn.org>
>>     Signed-off-by: David S. Miller <davem@davemloft.net>
>>
>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>
> On patches that have already been reviewed, accepted by the community
> and pushed upstream do we still ACK those here on the OVS dev list?

It's worth a last round of review just in case something was missed
upstream, but particularly if there are any changes within
datapath/linux/compat/* or outside datapath/, as those may not have
received review. The datapath backport compatibility
changes need to work across a variety of different kernels so
that's an easy vector to introduce bugs.
Jarno Rajahalme April 24, 2017, 9:22 p.m. UTC | #3
> On Apr 24, 2017, at 2:03 PM, Greg Rose <gvrose8192@gmail.com> wrote:
> 
> On Mon, 2017-04-24 at 13:50 -0700, Jarno Rajahalme wrote:
>> Upstream commit:
>> 
>>    commit 120645513f55a4ac5543120d9e79925d30a0156f
>>    Author: Jarno Rajahalme <jarno@ovn.org>
>>    Date:   Fri Apr 21 16:48:06 2017 -0700
>> 
>>    openvswitch: Add eventmask support to CT action.
>> 
>>    Add a new optional conntrack action attribute OVS_CT_ATTR_EVENTMASK,
>>    which can be used in conjunction with the commit flag
>>    (OVS_CT_ATTR_COMMIT) to set the mask of bits specifying which
>>    conntrack events (IPCT_*) should be delivered via the Netfilter
>>    netlink multicast groups.  Default behavior depends on the system
>>    configuration, but typically a lot of events are delivered.  This can be
>>    very chatty for the NFNLGRP_CONNTRACK_UPDATE group, even if only some
>>    types of events are of interest.
>> 
>>    Netfilter core init_conntrack() adds the event cache extension, so we
>>    only need to set the ctmask value.  However, if the system is
>>    configured without support for events, the setting will be skipped due
>>    to extension not being found.
>> 
>>    Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>>    Reviewed-by: Greg Rose <gvrose8192@gmail.com>
>>    Acked-by: Joe Stringer <joe@ovn.org>
>>    Signed-off-by: David S. Miller <davem@davemloft.net>
>> 
>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> 
> On patches that have already been reviewed, accepted by the community
> and pushed upstream do we still ACK those here on the OVS dev list?
> 

Yes we do, as sometimes OVS tree code is different due to backports to older kernels, OVS userspace build demands, and the openvswitch.h file is in different place. This one should be pretty straightforward, even though I had to apply the changes manually due to all of the above.

For backport review, it’s good to apply the patches, and build them and run the kernel testsuite at least on the kernel you are currently using for development, even if the changes seem innocent enough. I did the backport on Linux 3.16, but did not check it it compiles on anything else.

Sometimes backports require features missing from older supported kernels, so it can get quite complicated. You can find more info at Documentation/internals/contributing/backporting-patches.rst.

  Jarno


> Just want to keep noise down if they're not considered necessary.pa
> 
> Thanks,
> 
> - Greg
> 
>> ---
>> datapath/conntrack.c                              | 27 +++++++++++++++++++++++
>> datapath/linux/compat/include/linux/openvswitch.h | 12 ++++++++++
>> lib/dpif-netdev.c                                 |  4 ++++
>> 3 files changed, 43 insertions(+)
>> 
>> diff --git a/datapath/conntrack.c b/datapath/conntrack.c
>> index f911fe8..95c3739 100644
>> --- a/datapath/conntrack.c
>> +++ b/datapath/conntrack.c
>> @@ -73,7 +73,9 @@ struct ovs_conntrack_info {
>> 	u8 nat : 3;                 /* enum ovs_ct_nat */
>> 	u8 random_fully_compat : 1; /* bool */
>> 	u8 force : 1;
>> +	u8 have_eventmask : 1;
>> 	u16 family;
>> +	u32 eventmask;              /* Mask of 1 << IPCT_*. */
>> 	struct md_mark mark;
>> 	struct md_labels labels;
>> #ifdef CONFIG_NF_NAT_NEEDED
>> @@ -1041,6 +1043,20 @@ static int ovs_ct_commit(struct net *net, struct sw_flow_key *key,
>> 	if (!ct)
>> 		return 0;
>> 
>> +	/* Set the conntrack event mask if given.  NEW and DELETE events have
>> +	 * their own groups, but the NFNLGRP_CONNTRACK_UPDATE group listener
>> +	 * typically would receive many kinds of updates.  Setting the event
>> +	 * mask allows those events to be filtered.  The set event mask will
>> +	 * remain in effect for the lifetime of the connection unless changed
>> +	 * by a further CT action with both the commit flag and the eventmask
>> +	 * option. */
>> +	if (info->have_eventmask) {
>> +		struct nf_conntrack_ecache *cache = nf_ct_ecache_find(ct);
>> +
>> +		if (cache)
>> +			cache->ctmask = info->eventmask;
>> +	}
>> +
>> 	/* Apply changes before confirming the connection so that the initial
>> 	 * conntrack NEW netlink event carries the values given in the CT
>> 	 * action.
>> @@ -1277,6 +1293,8 @@ static const struct ovs_ct_len_tbl ovs_ct_attr_lens[OVS_CT_ATTR_MAX + 1] = {
>> 	/* NAT length is checked when parsing the nested attributes. */
>> 	[OVS_CT_ATTR_NAT]	= { .minlen = 0, .maxlen = INT_MAX },
>> #endif
>> +	[OVS_CT_ATTR_EVENTMASK]	= { .minlen = sizeof(u32),
>> +				    .maxlen = sizeof(u32) },
>> };
>> 
>> static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
>> @@ -1355,6 +1373,11 @@ static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
>> 			break;
>> 		}
>> #endif
>> +		case OVS_CT_ATTR_EVENTMASK:
>> +			info->have_eventmask = true;
>> +			info->eventmask = nla_get_u32(a);
>> +			break;
>> +
>> 		default:
>> 			OVS_NLERR(log, "Unknown conntrack attr (%d)",
>> 				  type);
>> @@ -1558,6 +1581,10 @@ int ovs_ct_action_to_attr(const struct ovs_conntrack_info *ct_info,
>> 				   ct_info->helper->name))
>> 			return -EMSGSIZE;
>> 	}
>> +	if (ct_info->have_eventmask &&
>> +	    nla_put_u32(skb, OVS_CT_ATTR_EVENTMASK, ct_info->eventmask))
>> +		return -EMSGSIZE;
>> +
>> #ifdef CONFIG_NF_NAT_NEEDED
>> 	if (ct_info->nat && !ovs_ct_nat_to_attr(ct_info, skb))
>> 		return -EMSGSIZE;
>> diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
>> index 8a6b729..72627f9 100644
>> --- a/datapath/linux/compat/include/linux/openvswitch.h
>> +++ b/datapath/linux/compat/include/linux/openvswitch.h
>> @@ -739,6 +739,17 @@ struct ovs_action_push_tnl {
>>  * nothing if the connection is already committed will check that the current
>>  * packet is in conntrack entry's original direction.  If directionality does
>>  * not match, will delete the existing conntrack entry and create a new one.
>> + * @OVS_CT_ATTR_EVENTMASK: Mask of bits indicating which conntrack event types
>> + * (enum ip_conntrack_events IPCT_*) should be reported.  For any bit set to
>> + * zero, the corresponding event type is not generated.  Default behavior
>> + * depends on system configuration, but typically all event types are
>> + * generated, hence listening on NFNLGRP_CONNTRACK_UPDATE events may get a lot
>> + * of events.  Explicitly passing this attribute allows limiting the updates
>> + * received to the events of interest.  The bit 1 << IPCT_NEW, 1 <<
>> + * IPCT_RELATED, and 1 << IPCT_DESTROY must be set to ones for those events to
>> + * be received on NFNLGRP_CONNTRACK_NEW and NFNLGRP_CONNTRACK_DESTROY groups,
>> + * respectively.  Remaining bits control the changes for which an event is
>> + * delivered on the NFNLGRP_CONNTRACK_UPDATE group.
>>  */
>> enum ovs_ct_attr {
>> 	OVS_CT_ATTR_UNSPEC,
>> @@ -750,6 +761,7 @@ enum ovs_ct_attr {
>> 				   related connections. */
>> 	OVS_CT_ATTR_NAT,        /* Nested OVS_NAT_ATTR_* */
>> 	OVS_CT_ATTR_FORCE_COMMIT,  /* No argument */
>> +	OVS_CT_ATTR_EVENTMASK,  /* u32 mask of IPCT_* events. */
>> 	__OVS_CT_ATTR_MAX
>> };
>> 
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 41d0836..b3a0806 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -5136,6 +5136,10 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
>>             case OVS_CT_ATTR_LABELS:
>>                 setlabel = nl_attr_get(b);
>>                 break;
>> +            case OVS_CT_ATTR_EVENTMASK:
>> +                /* Silently ignored, as userspace datapath does not generate
>> +                 * netlink events. */
>> +                break;
>>             case OVS_CT_ATTR_NAT:
>>             case OVS_CT_ATTR_UNSPEC:
>>             case __OVS_CT_ATTR_MAX:
> 
> 
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org <mailto:dev@openvswitch.org>
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
Joe Stringer April 27, 2017, 5:15 p.m. UTC | #4
On 24 April 2017 at 13:50, Jarno Rajahalme <jarno@ovn.org> wrote:
> Upstream commit:
>
>     commit 120645513f55a4ac5543120d9e79925d30a0156f
>     Author: Jarno Rajahalme <jarno@ovn.org>
>     Date:   Fri Apr 21 16:48:06 2017 -0700
>
>     openvswitch: Add eventmask support to CT action.
>
>     Add a new optional conntrack action attribute OVS_CT_ATTR_EVENTMASK,
>     which can be used in conjunction with the commit flag
>     (OVS_CT_ATTR_COMMIT) to set the mask of bits specifying which
>     conntrack events (IPCT_*) should be delivered via the Netfilter
>     netlink multicast groups.  Default behavior depends on the system
>     configuration, but typically a lot of events are delivered.  This can be
>     very chatty for the NFNLGRP_CONNTRACK_UPDATE group, even if only some
>     types of events are of interest.
>
>     Netfilter core init_conntrack() adds the event cache extension, so we
>     only need to set the ctmask value.  However, if the system is
>     configured without support for events, the setting will be skipped due
>     to extension not being found.
>
>     Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>     Reviewed-by: Greg Rose <gvrose8192@gmail.com>
>     Acked-by: Joe Stringer <joe@ovn.org>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
>
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>

Acked-by: Joe Stringer <joe@ovn.org>
Jarno Rajahalme April 27, 2017, 5:37 p.m. UTC | #5
> On Apr 27, 2017, at 10:15 AM, Joe Stringer <joe@ovn.org> wrote:
> 
> On 24 April 2017 at 13:50, Jarno Rajahalme <jarno@ovn.org> wrote:
>> Upstream commit:
>> 
>>    commit 120645513f55a4ac5543120d9e79925d30a0156f
>>    Author: Jarno Rajahalme <jarno@ovn.org>
>>    Date:   Fri Apr 21 16:48:06 2017 -0700
>> 
>>    openvswitch: Add eventmask support to CT action.
>> 
>>    Add a new optional conntrack action attribute OVS_CT_ATTR_EVENTMASK,
>>    which can be used in conjunction with the commit flag
>>    (OVS_CT_ATTR_COMMIT) to set the mask of bits specifying which
>>    conntrack events (IPCT_*) should be delivered via the Netfilter
>>    netlink multicast groups.  Default behavior depends on the system
>>    configuration, but typically a lot of events are delivered.  This can be
>>    very chatty for the NFNLGRP_CONNTRACK_UPDATE group, even if only some
>>    types of events are of interest.
>> 
>>    Netfilter core init_conntrack() adds the event cache extension, so we
>>    only need to set the ctmask value.  However, if the system is
>>    configured without support for events, the setting will be skipped due
>>    to extension not being found.
>> 
>>    Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>>    Reviewed-by: Greg Rose <gvrose8192@gmail.com>
>>    Acked-by: Joe Stringer <joe@ovn.org>
>>    Signed-off-by: David S. Miller <davem@davemloft.net>
>> 
>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> 
> Acked-by: Joe Stringer <joe@ovn.org>

Thanks for the review, series pushed to master.

  Jarno
diff mbox

Patch

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index f911fe8..95c3739 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -73,7 +73,9 @@  struct ovs_conntrack_info {
 	u8 nat : 3;                 /* enum ovs_ct_nat */
 	u8 random_fully_compat : 1; /* bool */
 	u8 force : 1;
+	u8 have_eventmask : 1;
 	u16 family;
+	u32 eventmask;              /* Mask of 1 << IPCT_*. */
 	struct md_mark mark;
 	struct md_labels labels;
 #ifdef CONFIG_NF_NAT_NEEDED
@@ -1041,6 +1043,20 @@  static int ovs_ct_commit(struct net *net, struct sw_flow_key *key,
 	if (!ct)
 		return 0;
 
+	/* Set the conntrack event mask if given.  NEW and DELETE events have
+	 * their own groups, but the NFNLGRP_CONNTRACK_UPDATE group listener
+	 * typically would receive many kinds of updates.  Setting the event
+	 * mask allows those events to be filtered.  The set event mask will
+	 * remain in effect for the lifetime of the connection unless changed
+	 * by a further CT action with both the commit flag and the eventmask
+	 * option. */
+	if (info->have_eventmask) {
+		struct nf_conntrack_ecache *cache = nf_ct_ecache_find(ct);
+
+		if (cache)
+			cache->ctmask = info->eventmask;
+	}
+
 	/* Apply changes before confirming the connection so that the initial
 	 * conntrack NEW netlink event carries the values given in the CT
 	 * action.
@@ -1277,6 +1293,8 @@  static const struct ovs_ct_len_tbl ovs_ct_attr_lens[OVS_CT_ATTR_MAX + 1] = {
 	/* NAT length is checked when parsing the nested attributes. */
 	[OVS_CT_ATTR_NAT]	= { .minlen = 0, .maxlen = INT_MAX },
 #endif
+	[OVS_CT_ATTR_EVENTMASK]	= { .minlen = sizeof(u32),
+				    .maxlen = sizeof(u32) },
 };
 
 static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
@@ -1355,6 +1373,11 @@  static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
 			break;
 		}
 #endif
+		case OVS_CT_ATTR_EVENTMASK:
+			info->have_eventmask = true;
+			info->eventmask = nla_get_u32(a);
+			break;
+
 		default:
 			OVS_NLERR(log, "Unknown conntrack attr (%d)",
 				  type);
@@ -1558,6 +1581,10 @@  int ovs_ct_action_to_attr(const struct ovs_conntrack_info *ct_info,
 				   ct_info->helper->name))
 			return -EMSGSIZE;
 	}
+	if (ct_info->have_eventmask &&
+	    nla_put_u32(skb, OVS_CT_ATTR_EVENTMASK, ct_info->eventmask))
+		return -EMSGSIZE;
+
 #ifdef CONFIG_NF_NAT_NEEDED
 	if (ct_info->nat && !ovs_ct_nat_to_attr(ct_info, skb))
 		return -EMSGSIZE;
diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
index 8a6b729..72627f9 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -739,6 +739,17 @@  struct ovs_action_push_tnl {
  * nothing if the connection is already committed will check that the current
  * packet is in conntrack entry's original direction.  If directionality does
  * not match, will delete the existing conntrack entry and create a new one.
+ * @OVS_CT_ATTR_EVENTMASK: Mask of bits indicating which conntrack event types
+ * (enum ip_conntrack_events IPCT_*) should be reported.  For any bit set to
+ * zero, the corresponding event type is not generated.  Default behavior
+ * depends on system configuration, but typically all event types are
+ * generated, hence listening on NFNLGRP_CONNTRACK_UPDATE events may get a lot
+ * of events.  Explicitly passing this attribute allows limiting the updates
+ * received to the events of interest.  The bit 1 << IPCT_NEW, 1 <<
+ * IPCT_RELATED, and 1 << IPCT_DESTROY must be set to ones for those events to
+ * be received on NFNLGRP_CONNTRACK_NEW and NFNLGRP_CONNTRACK_DESTROY groups,
+ * respectively.  Remaining bits control the changes for which an event is
+ * delivered on the NFNLGRP_CONNTRACK_UPDATE group.
  */
 enum ovs_ct_attr {
 	OVS_CT_ATTR_UNSPEC,
@@ -750,6 +761,7 @@  enum ovs_ct_attr {
 				   related connections. */
 	OVS_CT_ATTR_NAT,        /* Nested OVS_NAT_ATTR_* */
 	OVS_CT_ATTR_FORCE_COMMIT,  /* No argument */
+	OVS_CT_ATTR_EVENTMASK,  /* u32 mask of IPCT_* events. */
 	__OVS_CT_ATTR_MAX
 };
 
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 41d0836..b3a0806 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -5136,6 +5136,10 @@  dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
             case OVS_CT_ATTR_LABELS:
                 setlabel = nl_attr_get(b);
                 break;
+            case OVS_CT_ATTR_EVENTMASK:
+                /* Silently ignored, as userspace datapath does not generate
+                 * netlink events. */
+                break;
             case OVS_CT_ATTR_NAT:
             case OVS_CT_ATTR_UNSPEC:
             case __OVS_CT_ATTR_MAX: