[PATCH nf-next] netfilter: bridge: introduce broute meta statement

[PATCH nf-next] netfilter: bridge: introduce broute meta statement

[Sriram Yagnaraman]

nftables equivalent for ebtables -t broute.

Implement a new meta statement 'broute' to set br_netfilter_broute flag
in skb to force a packet to be routed instead of being bridged.

---
 include/uapi/linux/netfilter/nf_tables.h |  2 +
 net/bridge/netfilter/nft_meta_bridge.c   | 60 +++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index ff677f3a6cad..9c6f02c26054 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -931,6 +931,7 @@ enum nft_exthdr_attributes {
  * @NFT_META_TIME_HOUR: hour of day (in seconds)
  * @NFT_META_SDIF: slave device interface index
  * @NFT_META_SDIFNAME: slave device interface name
+ * @NFT_META_BRI_BROUTE: packet br_netfilter_broute bit
  */
 enum nft_meta_keys {
 	NFT_META_LEN,
@@ -969,6 +970,7 @@ enum nft_meta_keys {
 	NFT_META_TIME_HOUR,
 	NFT_META_SDIF,
 	NFT_META_SDIFNAME,
+	NFT_META_BRI_BROUTE,
 	__NFT_META_IIFTYPE,
 };
 
diff --git a/net/bridge/netfilter/nft_meta_bridge.c b/net/bridge/netfilter/nft_meta_bridge.c
index c3ecd77e25cb..2aa2d1dd89b1 100644
--- a/net/bridge/netfilter/nft_meta_bridge.c
+++ b/net/bridge/netfilter/nft_meta_bridge.c
@@ -8,6 +8,7 @@
 #include <net/netfilter/nf_tables.h>
 #include <net/netfilter/nft_meta.h>
 #include <linux/if_bridge.h>
+#include <../br_private.h>
 
 static const struct net_device *
 nft_meta_get_bridge(const struct net_device *dev)
@@ -102,6 +103,61 @@ static const struct nft_expr_ops nft_meta_bridge_get_ops = {
 	.reduce		= nft_meta_get_reduce,
 };
 
+void nft_meta_bridge_set_eval(const struct nft_expr *expr,
+			      struct nft_regs *regs,
+			      const struct nft_pktinfo *pkt)
+{
+	const struct nft_meta *meta = nft_expr_priv(expr);
+	struct sk_buff *skb = pkt->skb;
+	u32 *sreg = &regs->data[meta->sreg];
+	u8 value8;
+	struct net_bridge_port *p = br_port_get_rcu(skb->dev);
+	unsigned char *dest;
+
+	switch (meta->key) {
+	case NFT_META_BRI_BROUTE:
+		value8 = nft_reg_load8(sreg);
+
+		BR_INPUT_SKB_CB(skb)->br_netfilter_broute = !!value8;
+		/* undo PACKET_HOST mangling done in br_input in case the dst
+		 * address matches the logical bridge but not the port.
+		 */
+		dest = eth_hdr(skb)->h_dest;
+		if (skb->pkt_type == PACKET_HOST &&
+		    !ether_addr_equal(skb->dev->dev_addr, dest) &&
+		    ether_addr_equal(p->br->dev->dev_addr, dest))
+			skb->pkt_type = PACKET_OTHERHOST;
+		break;
+	default:
+		nft_meta_set_eval(expr, regs, pkt);
+	}
+}
+
+int nft_meta_bridge_set_init(const struct nft_ctx *ctx,
+			     const struct nft_expr *expr,
+			     const struct nlattr * const tb[])
+{
+	struct nft_meta *priv = nft_expr_priv(expr);
+	unsigned int len;
+	int err;
+
+	priv->key = ntohl(nla_get_be32(tb[NFTA_META_KEY]));
+	switch (priv->key) {
+	case NFT_META_BRI_BROUTE:
+		len = sizeof(u8);
+		break;
+	default:
+		return nft_meta_set_init(ctx, expr, tb);
+	}
+
+	priv->len = len;
+	err = nft_parse_register_load(tb[NFTA_META_SREG], &priv->sreg, len);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
 static bool nft_meta_bridge_set_reduce(struct nft_regs_track *track,
 				       const struct nft_expr *expr)
 {
@@ -123,8 +179,8 @@ static bool nft_meta_bridge_set_reduce(struct nft_regs_track *track,
 static const struct nft_expr_ops nft_meta_bridge_set_ops = {
 	.type		= &nft_meta_bridge_type,
 	.size		= NFT_EXPR_SIZE(sizeof(struct nft_meta)),
-	.eval		= nft_meta_set_eval,
-	.init		= nft_meta_set_init,
+	.eval		= nft_meta_bridge_set_eval,
+	.init		= nft_meta_bridge_set_init,
 	.destroy	= nft_meta_set_destroy,
 	.dump		= nft_meta_set_dump,
 	.reduce		= nft_meta_bridge_set_reduce,
-- 
2.34.1

Re: [PATCH nf-next] netfilter: bridge: introduce broute meta statement

[Jan Engelhardt]

On Thursday 2023-02-23 21:22, Sriram Yagnaraman wrote:
>+++ b/net/bridge/netfilter/nft_meta_bridge.c
>@@ -8,6 +8,7 @@
> #include <net/netfilter/nf_tables.h>
> #include <net/netfilter/nft_meta.h>
> #include <linux/if_bridge.h>
>+#include <../br_private.h>

This hurts my eye. At least, let's have "../br_private.h"?

Re: [PATCH nf-next] netfilter: bridge: introduce broute meta statement

[Florian Westphal]

Sriram Yagnaraman <sriram.yagnaraman@est.tech> wrote:
> +void nft_meta_bridge_set_eval(const struct nft_expr *expr,
> +			      struct nft_regs *regs,
> +			      const struct nft_pktinfo *pkt)

static?

> +{
> +		dest = eth_hdr(skb)->h_dest;
> +		if (skb->pkt_type == PACKET_HOST &&
> +		    !ether_addr_equal(skb->dev->dev_addr, dest) &&
> +		    ether_addr_equal(p->br->dev->dev_addr, dest))
> +			skb->pkt_type = PACKET_OTHERHOST;

We already support override of skb->pkt_type, I would prefer
if users to this explicitly from their ruleset if they need it.

> +	priv->key = ntohl(nla_get_be32(tb[NFTA_META_KEY]));

I think you need to check for !tb[NFTA_META_KEY] and bail out
before this line.

> +	switch (priv->key) {
> +	case NFT_META_BRI_BROUTE:
> +		len = sizeof(u8);
> +		break;

Can you bail out if this is called from something else
than PREROUTING hook?

You can look at nft_tproxy.c or similar on how to do this.

RE: [PATCH nf-next] netfilter: bridge: introduce broute meta statement

[Sriram Yagnaraman]

Hi Florian,

> -----Original Message-----
> From: Florian Westphal <fw@strlen.de>
> Sent: Friday, 24 February 2023 00:02
> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> Cc: netfilter-devel@vger.kernel.org; Florian Westphal <fw@strlen.de>; Pablo
> Neira Ayuso <pablo@netfilter.org>
> Subject: Re: [PATCH nf-next] netfilter: bridge: introduce broute meta
> statement
> 
> Sriram Yagnaraman <sriram.yagnaraman@est.tech> wrote:
> > +void nft_meta_bridge_set_eval(const struct nft_expr *expr,
> > +			      struct nft_regs *regs,
> > +			      const struct nft_pktinfo *pkt)
> 
> static?
> 
> > +{
> > +		dest = eth_hdr(skb)->h_dest;
> > +		if (skb->pkt_type == PACKET_HOST &&
> > +		    !ether_addr_equal(skb->dev->dev_addr, dest) &&
> > +		    ether_addr_equal(p->br->dev->dev_addr, dest))
> > +			skb->pkt_type = PACKET_OTHERHOST;
> 
> We already support override of skb->pkt_type, I would prefer if users to this
> explicitly from their ruleset if they need it.

Ok, that is better, I will remove this chunk.

> 
> > +	priv->key = ntohl(nla_get_be32(tb[NFTA_META_KEY]));
> 
> I think you need to check for !tb[NFTA_META_KEY] and bail out before this
> line.

We already validate this in nft_meta_bridge_select_ops(), isn’t that enough?

> 
> > +	switch (priv->key) {
> > +	case NFT_META_BRI_BROUTE:
> > +		len = sizeof(u8);
> > +		break;
> 
> Can you bail out if this is called from something else than PREROUTING hook?
> 
> You can look at nft_tproxy.c or similar on how to do this.

nft_meta_set_validate() already checks meta statements can only be used in the PREROUTING hook. Isn't that enough?

Re: [PATCH nf-next] netfilter: bridge: introduce broute meta statement

[Florian Westphal]

Sriram Yagnaraman <sriram.yagnaraman@est.tech> wrote:
> > -----Original Message-----
> > From: Florian Westphal <fw@strlen.de>
> > Sent: Friday, 24 February 2023 00:02
> > To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> > Cc: netfilter-devel@vger.kernel.org; Florian Westphal <fw@strlen.de>; Pablo
> > Neira Ayuso <pablo@netfilter.org>
> > Subject: Re: [PATCH nf-next] netfilter: bridge: introduce broute meta
> > statement
> > 
> > Sriram Yagnaraman <sriram.yagnaraman@est.tech> wrote:
> > > +void nft_meta_bridge_set_eval(const struct nft_expr *expr,
> > > +			      struct nft_regs *regs,
> > > +			      const struct nft_pktinfo *pkt)
> > 
> > static?
> > 
> > > +{
> > > +		dest = eth_hdr(skb)->h_dest;
> > > +		if (skb->pkt_type == PACKET_HOST &&
> > > +		    !ether_addr_equal(skb->dev->dev_addr, dest) &&
> > > +		    ether_addr_equal(p->br->dev->dev_addr, dest))
> > > +			skb->pkt_type = PACKET_OTHERHOST;
> > 
> > We already support override of skb->pkt_type, I would prefer if users to this
> > explicitly from their ruleset if they need it.
> 
> Ok, that is better, I will remove this chunk.
> 
> > 
> > > +	priv->key = ntohl(nla_get_be32(tb[NFTA_META_KEY]));
> > 
> > I think you need to check for !tb[NFTA_META_KEY] and bail out before this
> > line.
> 
> We already validate this in nft_meta_bridge_select_ops(), isn’t that enough?

Right, thats enough.

> > > +	switch (priv->key) {
> > > +	case NFT_META_BRI_BROUTE:
> > > +		len = sizeof(u8);
> > > +		break;
> > 
> > Can you bail out if this is called from something else than PREROUTING hook?
> > 
> > You can look at nft_tproxy.c or similar on how to do this.
> 
> nft_meta_set_validate() already checks meta statements can only be used in the PREROUTING hook. Isn't that enough?

It only restricts NFT_META_PKTTYPE.