Thanks Joe, I’ll issue a v2 with the comment fix and retain the acks, so it
should be good to go.
Jarno
> On Apr 20, 2017, at 11:53 AM, Joe Stringer <j...@ovn.org> wrote:
>
> On 19 April 2017 at 18:49, Jarno Rajahalme <ja...@ovn.org> 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>
>> ---
>
> Thanks Jarno, looks good overall. Minor nit in the API description below.
>
> Acked-by: Joe Stringer <j...@ovn.org>
>
>> 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.
>
> s/UPDATE/NFNLGRP_CONNTRACK_UPDATE/
>
>> + * 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;
>> --
>> 2.1.4
