On Wed, 2017-04-19 at 18:55 -0700, Jarno Rajahalme wrote:
> 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 <ja...@ovn.org>
Looks good to me but I'll let Joe ack it since I'm not that familiar
with the code yet. Nevertheless...
Reviewed-by: Greg Rose <gvrose@8...@gmail.com>
> ---
> include/uapi/linux/openvswitch.h | 12 ++++++++++++
> net/openvswitch/conntrack.c | 27 +++++++++++++++++++++++++++
> 2 files changed, 39 insertions(+)
>
> diff --git a/include/uapi/linux/openvswitch.h
> b/include/uapi/linux/openvswitch.h
> index 66d1c3c..38ae95d 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -693,6 +693,17 @@ struct ovs_action_hash {
> * 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 commit 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 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,
> @@ -704,6 +715,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/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 58de4c2..4f7c3b5 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -66,7 +66,9 @@ struct ovs_conntrack_info {
> u8 commit : 1;
> u8 nat : 3; /* enum ovs_ct_nat */
> 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
> @@ -1007,6 +1009,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.
> @@ -1238,6 +1254,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,
> @@ -1316,6 +1334,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);
> @@ -1515,6 +1538,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;
