Re: [PATCHv4 net-next 10/10] openvswitch: Allow attaching helpers to ct action

On 19 August 2015 at 15:57, Pravin Shelar [off-list ref] wrote:

On Tue, Aug 18, 2015 at 4:39 PM, Joe Stringer [off-list ref] wrote:

quoted

Add support for using conntrack helpers to assist protocol detection.
The new OVS_CT_ATTR_HELPER attribute of the ct action specifies a helper
to be used for this connection.

Example ODP flows allowing FTP connections from ports 1->2:
in_port=1,tcp,action=ct(helper=ftp,commit),2
in_port=2,tcp,ct_state=-trk,action=ct(),recirc(1)
recirc_id=1,in_port=2,tcp,ct_state=+trk-new+est,action=1
recirc_id=1,in_port=2,tcp,ct_state=+trk+rel,action=1

Signed-off-by: Joe Stringer <redacted>
---
v2-v3: No change.
v4: Change error code for unknown helper ENOENT->EINVAL.

I got following compilation warning :

net/openvswitch/conntrack.c:352:42: error: incompatible types in
comparison expression (different address spaces)

Is this made available via another sparse flag? It looks like it's
related to the __rcu as you've mentioned below, but I'm not seeing
this (latest sparse, gcc-4.9.2)

quoted

+static int ovs_ct_add_helper(struct ovs_conntrack_info *info, const char *name,
+                            const struct sw_flow_key *key, bool log)
+{
+       struct nf_conntrack_helper *helper;
+       struct nf_conn_help *help;
+
+       helper = nf_conntrack_helper_try_module_get(name, info->family,
+                                                   key->ip.proto);
+       if (!helper) {
+               OVS_NLERR(log, "Unknown helper \"%s\"", name);
+               return -EINVAL;
+       }
+
+       help = nf_ct_helper_ext_add(info->ct, helper, GFP_KERNEL);
+       if (!help) {
+               module_put(helper->me);
+               return -ENOMEM;
+       }
+
+       help->helper = helper;

helper is rcu pointer so need to use rcu API to set the value. I know
it is not required here, but it is still cleaner to use the API.

Will update, thanks.