Hi Holger,
I like patches 1/3 and 2/3, they are nice cleanups.
Some comments regarding this patch.
On Thu, Sep 26, 2013 at 05:31:53PM +0200, Holger Eitzenberger wrote:
> The interface indices are exported as uint32_t, although being
> signed integer inside the kernel, which goes in line with
> what nfnetlink_queue does.
>
> Both interface indices are wrapped inside CTA_ACCT.
>
> Signed-off-by: Holger Eitzenberger <holger@xxxxxxxxxxxxxxxx>
>
> Index: net-next-ipfix/include/net/netfilter/nf_conntrack_acct.h
> ===================================================================
> --- net-next-ipfix.orig/include/net/netfilter/nf_conntrack_acct.h
> +++ net-next-ipfix/include/net/netfilter/nf_conntrack_acct.h
> @@ -21,6 +21,8 @@ struct nf_conn_counter {
>
> struct nf_conn_acct {
> struct nf_conn_counter counter[IP_CT_DIR_MAX];
> + int indev;
> + int outdev;
> };
>
> static inline
> Index: net-next-ipfix/net/netfilter/nf_conntrack_core.c
> ===================================================================
> --- net-next-ipfix.orig/net/netfilter/nf_conntrack_core.c
> +++ net-next-ipfix/net/netfilter/nf_conntrack_core.c
> @@ -33,6 +33,7 @@
> #include <linux/mm.h>
> #include <linux/nsproxy.h>
> #include <linux/rculist_nulls.h>
> +#include <net/dst.h>
>
> #include <net/netfilter/nf_conntrack.h>
> #include <net/netfilter/nf_conntrack_l3proto.h>
> @@ -1110,6 +1111,7 @@ void __nf_ct_refresh_acct(struct nf_conn
> acct:
> if (do_acct) {
> struct nf_conn_acct *acct;
> + struct dst_entry *dst;
>
> acct = nf_conn_acct_find(ct);
> if (acct) {
> @@ -1117,6 +1119,13 @@ acct:
>
> atomic64_inc(&counter[CTINFO2DIR(ctinfo)].packets);
> atomic64_add(skb->len, &counter[CTINFO2DIR(ctinfo)].bytes);
> +
> + if (acct->indev == 0 && skb->dev)
> + acct->indev = skb->dev->ifindex;
> +
> + dst = skb_dst(skb);
> + if (acct->outdev == 0 && dst && dst->dev)
> + acct->outdev = dst->dev->ifindex;
If you only set indev/outdev once we can skip the conntrack extension
by passing the skb to nf_ct_deliver_cached_events and include this
information in the conntrack events. That would not allow to dump the
device from conntrack dumps though. I still have concerns with this
approach as this doesn't seem to cover the scenario in which the
in/outdev changes.
Regards.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
