Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] netfilter: nft_fwd_netdev: Fix CONFIG_NET_CLS_ACT=n build
@ 2020-03-25  9:33 Geert Uytterhoeven
  2020-03-25 10:40 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 4+ messages in thread
From: Geert Uytterhoeven @ 2020-03-25  9:33 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	David S . Miller, Jakub Kicinski
  Cc: netfilter-devel, coreteam, netdev, linux-next, Geert Uytterhoeven

If CONFIG_NET_CLS_ACT=n:

    net/netfilter/nft_fwd_netdev.c: In function ‘nft_fwd_netdev_eval’:
    net/netfilter/nft_fwd_netdev.c:32:10: error: ‘struct sk_buff’ has no member named ‘tc_redirected’
      pkt->skb->tc_redirected = 1;
	      ^~
    net/netfilter/nft_fwd_netdev.c:33:10: error: ‘struct sk_buff’ has no member named ‘tc_from_ingress’
      pkt->skb->tc_from_ingress = 1;
	      ^~

Fix this by protecting this code hunk with the appropriate #ifdef.

Reported-by: noreply@ellerman.id.au
Fixes: bcfabee1afd99484 ("netfilter: nft_fwd_netdev: allow to redirect to ifb via ingress")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 net/netfilter/nft_fwd_netdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/nft_fwd_netdev.c b/net/netfilter/nft_fwd_netdev.c
index 74f050ba6badc9dc..ebcaf5c325712f30 100644
--- a/net/netfilter/nft_fwd_netdev.c
+++ b/net/netfilter/nft_fwd_netdev.c
@@ -28,9 +28,11 @@ static void nft_fwd_netdev_eval(const struct nft_expr *expr,
 	struct nft_fwd_netdev *priv = nft_expr_priv(expr);
 	int oif = regs->data[priv->sreg_dev];
 
+#ifdef CONFIG_NET_CLS_ACT
 	/* These are used by ifb only. */
 	pkt->skb->tc_redirected = 1;
 	pkt->skb->tc_from_ingress = 1;
+#endif
 
 	nf_fwd_netdev_egress(pkt, oif);
 	regs->verdict.code = NF_STOLEN;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] netfilter: nft_fwd_netdev: Fix CONFIG_NET_CLS_ACT=n build
  2020-03-25  9:33 [PATCH] netfilter: nft_fwd_netdev: Fix CONFIG_NET_CLS_ACT=n build Geert Uytterhoeven
@ 2020-03-25 10:40 ` Pablo Neira Ayuso
  2020-03-25 11:00   ` Geert Uytterhoeven
  0 siblings, 1 reply; 4+ messages in thread
From: Pablo Neira Ayuso @ 2020-03-25 10:40 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jozsef Kadlecsik, Florian Westphal, David S . Miller,
	Jakub Kicinski, netfilter-devel, coreteam, netdev, linux-next

[-- Attachment #1: Type: text/plain, Size: 780 bytes --]

On Wed, Mar 25, 2020 at 10:33:00AM +0100, Geert Uytterhoeven wrote:
> If CONFIG_NET_CLS_ACT=n:
> 
>     net/netfilter/nft_fwd_netdev.c: In function ‘nft_fwd_netdev_eval’:
>     net/netfilter/nft_fwd_netdev.c:32:10: error: ‘struct sk_buff’ has no member named ‘tc_redirected’
>       pkt->skb->tc_redirected = 1;
> 	      ^~
>     net/netfilter/nft_fwd_netdev.c:33:10: error: ‘struct sk_buff’ has no member named ‘tc_from_ingress’
>       pkt->skb->tc_from_ingress = 1;
> 	      ^~
> 
> Fix this by protecting this code hunk with the appropriate #ifdef.

Sorry about this, and thank you for fixing up this so fast.

I'm attaching an alternative fix to avoid a dependency on tc from
netfilter. Still testing, if fine and no objections I'll formally
submit this.

[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 5419 bytes --]

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 25a8f9387d5a..db8884ad6d40 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -149,6 +149,7 @@ config NET_FC
 config IFB
 	tristate "Intermediate Functional Block support"
 	depends on NET_CLS_ACT
+	select NET_REDIRECT
 	---help---
 	  This is an intermediate driver that allows sharing of
 	  resources.
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 242b9b0943f8..7fe306e76281 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -75,7 +75,7 @@ static void ifb_ri_tasklet(unsigned long _txp)
 	}
 
 	while ((skb = __skb_dequeue(&txp->tq)) != NULL) {
-		skb->tc_redirected = 0;
+		skb->redirected = 0;
 		skb->tc_skip_classify = 1;
 
 		u64_stats_update_begin(&txp->tsync);
@@ -96,7 +96,7 @@ static void ifb_ri_tasklet(unsigned long _txp)
 		rcu_read_unlock();
 		skb->skb_iif = txp->dev->ifindex;
 
-		if (!skb->tc_from_ingress) {
+		if (!skb->from_ingress) {
 			dev_queue_xmit(skb);
 		} else {
 			skb_pull_rcsum(skb, skb->mac_len);
@@ -243,7 +243,7 @@ static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
 	txp->rx_bytes += skb->len;
 	u64_stats_update_end(&txp->rsync);
 
-	if (!skb->tc_redirected || !skb->skb_iif) {
+	if (!skb->redirected || !skb->skb_iif) {
 		dev_kfree_skb(skb);
 		dev->stats.rx_dropped++;
 		return NETDEV_TX_OK;
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 5b50278c4bc8..42e1ffc25d32 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -848,8 +848,10 @@ struct sk_buff {
 #ifdef CONFIG_NET_CLS_ACT
 	__u8			tc_skip_classify:1;
 	__u8			tc_at_ingress:1;
-	__u8			tc_redirected:1;
-	__u8			tc_from_ingress:1;
+#endif
+#ifdef CONFIG_NET_REDIRECT
+	__u8			redirected:1;
+	__u8			from_ingress:1;
 #endif
 #ifdef CONFIG_TLS_DEVICE
 	__u8			decrypted:1;
@@ -4579,5 +4581,31 @@ static inline __wsum lco_csum(struct sk_buff *skb)
 	return csum_partial(l4_hdr, csum_start - l4_hdr, partial);
 }
 
+static inline bool skb_is_redirected(const struct sk_buff *skb)
+{
+#ifdef CONFIG_NET_REDIRECT
+	return skb->redirected;
+#else
+	return false;
+#endif
+}
+
+static inline void skb_set_redirected(struct sk_buff *skb, bool from_ingress)
+{
+#ifdef CONFIG_NET_REDIRECT
+	skb->redirected = 1;
+	skb->from_ingress = from_ingress;
+	if (skb->from_ingress)
+		skb->tstamp = 0;
+#endif
+}
+
+static inline void skb_reset_redirect(struct sk_buff *skb)
+{
+#ifdef CONFIG_NET_REDIRECT
+	skb->redirected = 0;
+#endif
+}
+
 #endif	/* __KERNEL__ */
 #endif	/* _LINUX_SKBUFF_H */
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 151208704ed2..c30f914867e6 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -675,22 +675,6 @@ void __qdisc_calculate_pkt_len(struct sk_buff *skb,
 			       const struct qdisc_size_table *stab);
 int skb_do_redirect(struct sk_buff *);
 
-static inline void skb_reset_tc(struct sk_buff *skb)
-{
-#ifdef CONFIG_NET_CLS_ACT
-	skb->tc_redirected = 0;
-#endif
-}
-
-static inline bool skb_is_tc_redirected(const struct sk_buff *skb)
-{
-#ifdef CONFIG_NET_CLS_ACT
-	return skb->tc_redirected;
-#else
-	return false;
-#endif
-}
-
 static inline bool skb_at_tc_ingress(const struct sk_buff *skb)
 {
 #ifdef CONFIG_NET_CLS_ACT
diff --git a/net/Kconfig b/net/Kconfig
index 2eeb0e55f7c9..df8d8c9bd021 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -52,6 +52,9 @@ config NET_INGRESS
 config NET_EGRESS
 	bool
 
+config NET_REDIRECT
+	bool
+
 config SKB_EXTENSIONS
 	bool
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 402a986659cf..500bba8874b0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4516,7 +4516,7 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
 	/* Reinjected packets coming from act_mirred or similar should
 	 * not get XDP generic processing.
 	 */
-	if (skb_is_tc_redirected(skb))
+	if (skb_is_redirected(skb))
 		return XDP_PASS;
 
 	/* XDP packets must be linear and must have sufficient headroom
@@ -5063,7 +5063,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc,
 			goto out;
 	}
 #endif
-	skb_reset_tc(skb);
+	skb_reset_redirect(skb);
 skip_classify:
 	if (pfmemalloc && !skb_pfmemalloc_protocol(skb))
 		goto drop;
diff --git a/net/netfilter/nft_fwd_netdev.c b/net/netfilter/nft_fwd_netdev.c
index 74f050ba6bad..3087e23297db 100644
--- a/net/netfilter/nft_fwd_netdev.c
+++ b/net/netfilter/nft_fwd_netdev.c
@@ -28,9 +28,8 @@ static void nft_fwd_netdev_eval(const struct nft_expr *expr,
 	struct nft_fwd_netdev *priv = nft_expr_priv(expr);
 	int oif = regs->data[priv->sreg_dev];
 
-	/* These are used by ifb only. */
-	pkt->skb->tc_redirected = 1;
-	pkt->skb->tc_from_ingress = 1;
+	/* This is used by ifb only. */
+	skb_set_redirected(pkt->skb, true);
 
 	nf_fwd_netdev_egress(pkt, oif);
 	regs->verdict.code = NF_STOLEN;
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 1ad300e6dbc0..83dd82fc9f40 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -284,10 +284,8 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
 
 	/* mirror is always swallowed */
 	if (is_redirect) {
-		skb2->tc_redirected = 1;
-		skb2->tc_from_ingress = skb2->tc_at_ingress;
-		if (skb2->tc_from_ingress)
-			skb2->tstamp = 0;
+		skb_set_redirected(skb2, skb2->tc_at_ingress);
+
 		/* let's the caller reinsert the packet, if possible */
 		if (use_reinsert) {
 			res->ingress = want_ingress;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] netfilter: nft_fwd_netdev: Fix CONFIG_NET_CLS_ACT=n build
  2020-03-25 10:40 ` Pablo Neira Ayuso
@ 2020-03-25 11:00   ` Geert Uytterhoeven
  2020-03-25 11:02     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 4+ messages in thread
From: Geert Uytterhoeven @ 2020-03-25 11:00 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Jozsef Kadlecsik, Florian Westphal, David S . Miller,
	Jakub Kicinski, NetFilter, coreteam, netdev, Linux-Next

Hi Pablo,

On Wed, Mar 25, 2020 at 11:40 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Mar 25, 2020 at 10:33:00AM +0100, Geert Uytterhoeven wrote:
> > If CONFIG_NET_CLS_ACT=n:
> >
> >     net/netfilter/nft_fwd_netdev.c: In function ‘nft_fwd_netdev_eval’:
> >     net/netfilter/nft_fwd_netdev.c:32:10: error: ‘struct sk_buff’ has no member named ‘tc_redirected’
> >       pkt->skb->tc_redirected = 1;
> >             ^~
> >     net/netfilter/nft_fwd_netdev.c:33:10: error: ‘struct sk_buff’ has no member named ‘tc_from_ingress’
> >       pkt->skb->tc_from_ingress = 1;
> >             ^~
> >
> > Fix this by protecting this code hunk with the appropriate #ifdef.
>
> Sorry about this, and thank you for fixing up this so fast.
>
> I'm attaching an alternative fix to avoid a dependency on tc from
> netfilter. Still testing, if fine and no objections I'll formally
> submit this.

Thanks, that fixes the issue, too.
(Note that I didn't do a full build).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] netfilter: nft_fwd_netdev: Fix CONFIG_NET_CLS_ACT=n build
  2020-03-25 11:00   ` Geert Uytterhoeven
@ 2020-03-25 11:02     ` Pablo Neira Ayuso
  0 siblings, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2020-03-25 11:02 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jozsef Kadlecsik, Florian Westphal, David S . Miller,
	Jakub Kicinski, NetFilter, coreteam, netdev, Linux-Next

On Wed, Mar 25, 2020 at 12:00:08PM +0100, Geert Uytterhoeven wrote:
> Hi Pablo,
> 
> On Wed, Mar 25, 2020 at 11:40 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Wed, Mar 25, 2020 at 10:33:00AM +0100, Geert Uytterhoeven wrote:
> > > If CONFIG_NET_CLS_ACT=n:
> > >
> > >     net/netfilter/nft_fwd_netdev.c: In function ‘nft_fwd_netdev_eval’:
> > >     net/netfilter/nft_fwd_netdev.c:32:10: error: ‘struct sk_buff’ has no member named ‘tc_redirected’
> > >       pkt->skb->tc_redirected = 1;
> > >             ^~
> > >     net/netfilter/nft_fwd_netdev.c:33:10: error: ‘struct sk_buff’ has no member named ‘tc_from_ingress’
> > >       pkt->skb->tc_from_ingress = 1;
> > >             ^~
> > >
> > > Fix this by protecting this code hunk with the appropriate #ifdef.
> >
> > Sorry about this, and thank you for fixing up this so fast.
> >
> > I'm attaching an alternative fix to avoid a dependency on tc from
> > netfilter. Still testing, if fine and no objections I'll formally
> > submit this.
> 
> Thanks, that fixes the issue, too.
> (Note that I didn't do a full build).

Thanks. I'll submit formaly asap.

There was another spot I forgot to update in net/core/pktgen.c, I will
include that chunk too in my formal submission.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-25  9:33 [PATCH] netfilter: nft_fwd_netdev: Fix CONFIG_NET_CLS_ACT=n build Geert Uytterhoeven
2020-03-25 10:40 ` Pablo Neira Ayuso
2020-03-25 11:00   ` Geert Uytterhoeven
2020-03-25 11:02     ` Pablo Neira Ayuso

Netfilter-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \
		netfilter-devel@vger.kernel.org
	public-inbox-index netfilter-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git