Re: [PATCH nf-next RFC,v2 4/6] netfilter: flow table support for IPv4

On Fri, Dec 08, 2017 at 11:04:13AM +0100, Florian Westphal wrote:

Pablo Neira Ayuso [off-list ref] wrote:

quoted

This patch adds the IPv4 flow table type, that implements the datapath
flow table to forward IPv4 traffic. Rationale is:

1) Look up for the packet in the flow table, from the ingress hook.
2) If there's a hit, decrement ttl and pass it on to the neighbour layer
   for transmission.
3) If there's a miss, packet is passed up to the classic forwarding
   path.

Is there a plan to also handle zone IDs in future?

Zone ID is meaningful to whoever applies the policy: in this offload
approach this patchset implements, the policy resides in the kernel.

I't going to be messy for sure since we'd need to tell HW how to do
the zone mapping.  Perhaps only support a builtin list, e.g.
vlan id == zone...?

I've been considering a more simple solution, ie. add the input ifindex
device in the flowtable hash lookup, as part of the flow tuple. All
examples I've been observing for zones are basically mapping network
interfaces to zones.

Don't yet see how it could be done in a generic way as the mappings can
be arbitrarily complex.

Right now afaics one could install one flow table per zone and map
this in nft, but then we still miss the part that tells the hardware
how the zone identifier was derived.

quoted

+static bool ip_has_options(unsigned int thoff)
+{
+	return thoff > sizeof(struct iphdr);

I'd use
	thoff != sizeof(...)

to catch case where ihl is < struct iphdr.

ok.

quoted

+nf_flow_offload_hook(void *priv, struct sk_buff *skb,
+		     const struct nf_hook_state *state)
+{
+	struct flow_offload_tuple_rhash *tuplehash;
+	struct nf_flowtable *flow_table = priv;
+	struct flow_offload_tuple tuple = {};
+	union nf_inet_addr nexthop;
+	struct flow_offload *flow;
+	struct net_device *outdev;
+	struct iphdr *iph;
+
+	if (nf_flow_tuple_ip(skb, &tuple) < 0)
+		return NF_ACCEPT;
+
+	tuplehash = flow_offload_lookup(flow_table, &tuple);
+	if (tuplehash == NULL)
+		return NF_ACCEPT;
+
+	outdev = dev_get_by_index_rcu(&init_net, tuplehash->tuple.oifidx);

state->net ?

Yes, netns support is in my TODO list.