* [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic
@ 2009-11-04 19:05 Bart De Schuymer
2009-11-05 14:32 ` Patrick McHardy
0 siblings, 1 reply; 12+ messages in thread
From: Bart De Schuymer @ 2009-11-04 19:05 UTC (permalink / raw)
To: Netfilter Developer Mailing List
[-- Attachment #1: Type: text/plain, Size: 572 bytes --]
Hi,
The attached patch does the following:
1. fix a bug introduced in commit
9d02002d2dc2c7423e5891b97727fde4d667adf1 (2/10/2006) which made
ipt_REJECT stop work for bridged traffic (use of nskb instead of oldskb)
2. use the correct source MAC address for the response (bug reported in
bug 531 of netfilter's bugzilla)
Tested for plain IP traffic and IP traffic encapsulated inside a VLAN
header (should also work for PPPoE encapsulated IP traffic).
Signed-off-by: Bart De Schuymer <bdschuym@pandora.be>
cheers,
Bart
--
Bart De Schuymer
www.artinalgorithms.be
[-- Attachment #2: ipt_REJECT_use_correct_SMAC.diff --]
[-- Type: text/plain, Size: 2814 bytes --]
--- linux-2.6.31-uml/include/linux/netfilter_bridge.h.ori 2009-11-02 20:58:59.000000000 +0100
+++ linux-2.6.31-uml/include/linux/netfilter_bridge.h 2009-11-02 19:58:09.000000000 +0100
@@ -44,6 +44,7 @@ enum nf_br_hook_priorities {
#define BRNF_DONT_TAKE_PARENT 0x04
#define BRNF_BRIDGED 0x08
#define BRNF_NF_BRIDGE_PREROUTING 0x10
+#define BRNF_COPY_MAC_SADDR 0x20
/* Only used in br_forward.c */
@@ -77,6 +78,15 @@ static inline unsigned int nf_bridge_pad
return 0;
}
+static inline struct nf_bridge_info *nf_bridge_alloc(struct sk_buff *skb)
+{
+ skb->nf_bridge = kzalloc(sizeof(struct nf_bridge_info), GFP_ATOMIC);
+ if (likely(skb->nf_bridge))
+ atomic_set(&(skb->nf_bridge->use), 1);
+
+ return skb->nf_bridge;
+}
+
struct bridge_skb_cb {
union {
__be32 ipv4;
--- linux-2.6.31-uml/net/bridge/br_netfilter.c.fixed 2009-11-02 21:22:00.000000000 +0100
+++ linux-2.6.31-uml/net/bridge/br_netfilter.c 2009-11-03 22:18:41.000000000 +0100
@@ -145,15 +145,6 @@ static inline struct net_device *bridge_
return port ? port->br->dev : NULL;
}
-static inline struct nf_bridge_info *nf_bridge_alloc(struct sk_buff *skb)
-{
- skb->nf_bridge = kzalloc(sizeof(struct nf_bridge_info), GFP_ATOMIC);
- if (likely(skb->nf_bridge))
- atomic_set(&(skb->nf_bridge->use), 1);
-
- return skb->nf_bridge;
-}
-
static inline struct nf_bridge_info *nf_bridge_unshare(struct sk_buff *skb)
{
struct nf_bridge_info *nf_bridge = skb->nf_bridge;
@@ -775,6 +766,13 @@ static unsigned int br_nf_local_out(unsi
return NF_DROP;
nf_bridge = skb->nf_bridge;
+ /* Enable complete transparency for e.g. ipt_REJECT */
+ if (nf_bridge->mask & BRNF_COPY_MAC_SADDR) {
+ skb_copy_to_linear_data_offset(skb, -8, nf_bridge->data, 6);
+ nf_bridge_put(nf_bridge);
+ skb->nf_bridge = NULL;
+ return NF_ACCEPT;
+ }
if (!(nf_bridge->mask & BRNF_BRIDGED_DNAT))
return NF_ACCEPT;
--- linux-2.6.31-uml/net/ipv4/netfilter/ipt_REJECT.c.ori 2009-10-31 19:31:54.000000000 +0100
+++ linux-2.6.31-uml/net/ipv4/netfilter/ipt_REJECT.c 2009-11-03 21:55:08.000000000 +0100
@@ -100,11 +100,19 @@ static void send_reset(struct sk_buff *o
sizeof(struct tcphdr), 0));
addr_type = RTN_UNSPEC;
- if (hook != NF_INET_FORWARD
#ifdef CONFIG_BRIDGE_NETFILTER
- || (nskb->nf_bridge && nskb->nf_bridge->mask & BRNF_BRIDGED)
+ if (oldskb->nf_bridge && oldskb->nf_bridge->mask & BRNF_BRIDGED) {
+ int daddr_offset = -14 - nf_bridge_encap_header_len(oldskb);
+
+ addr_type = RTN_LOCAL;
+ if (!nf_bridge_alloc(nskb))
+ goto free_nskb;
+ nskb->nf_bridge->mask |= BRNF_COPY_MAC_SADDR;
+ skb_copy_from_linear_data_offset(oldskb, daddr_offset,
+ nskb->nf_bridge->data, 6);
+ } else
#endif
- )
+ if (hook != NF_INET_FORWARD)
addr_type = RTN_LOCAL;
/* ip_route_me_harder expects skb->dst to be set */
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic
2009-11-04 19:05 [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic Bart De Schuymer
@ 2009-11-05 14:32 ` Patrick McHardy
2009-11-05 19:19 ` Bart De Schuymer
0 siblings, 1 reply; 12+ messages in thread
From: Patrick McHardy @ 2009-11-05 14:32 UTC (permalink / raw)
To: Bart De Schuymer; +Cc: Netfilter Developer Mailing List
Bart De Schuymer wrote:
> Hi,
>
> The attached patch does the following:
> 1. fix a bug introduced in commit
> 9d02002d2dc2c7423e5891b97727fde4d667adf1 (2/10/2006) which made
> ipt_REJECT stop work for bridged traffic (use of nskb instead of oldskb)
> 2. use the correct source MAC address for the response (bug reported in
> bug 531 of netfilter's bugzilla)
>
> Tested for plain IP traffic and IP traffic encapsulated inside a VLAN
> header (should also work for PPPoE encapsulated IP traffic).
>
>
> --- linux-2.6.31-uml/net/bridge/br_netfilter.c.fixed 2009-11-02 21:22:00.000000000 +0100
> +++ linux-2.6.31-uml/net/bridge/br_netfilter.c 2009-11-03 22:18:41.000000000 +0100
> @@ -775,6 +766,13 @@ static unsigned int br_nf_local_out(unsi
> return NF_DROP;
>
> nf_bridge = skb->nf_bridge;
> + /* Enable complete transparency for e.g. ipt_REJECT */
> + if (nf_bridge->mask & BRNF_COPY_MAC_SADDR) {
> + skb_copy_to_linear_data_offset(skb, -8, nf_bridge->data, 6);
Please use the proper ETH_*LEN values. I guess that would be
skb_copy_to_linear_data_offset(skb, -(ETH_HLEN - ETH_ALEN),
nf_bridge->data, ETH_ALEN)
> + nf_bridge_put(nf_bridge);
> + skb->nf_bridge = NULL;
> + return NF_ACCEPT;
Shouldn't packets with BRNF_BRIDGED_DNAT continue through NF_BR_FORWARD
like they used to?
> + }
> if (!(nf_bridge->mask & BRNF_BRIDGED_DNAT))
> return NF_ACCEPT;
>
> --- linux-2.6.31-uml/net/ipv4/netfilter/ipt_REJECT.c.ori 2009-10-31 19:31:54.000000000 +0100
> +++ linux-2.6.31-uml/net/ipv4/netfilter/ipt_REJECT.c 2009-11-03 21:55:08.000000000 +0100
> @@ -100,11 +100,19 @@ static void send_reset(struct sk_buff *o
> sizeof(struct tcphdr), 0));
>
> addr_type = RTN_UNSPEC;
> - if (hook != NF_INET_FORWARD
> #ifdef CONFIG_BRIDGE_NETFILTER
> - || (nskb->nf_bridge && nskb->nf_bridge->mask & BRNF_BRIDGED)
> + if (oldskb->nf_bridge && oldskb->nf_bridge->mask & BRNF_BRIDGED) {
> + int daddr_offset = -14 - nf_bridge_encap_header_len(oldskb);
> +
> + addr_type = RTN_LOCAL;
> + if (!nf_bridge_alloc(nskb))
> + goto free_nskb;
> + nskb->nf_bridge->mask |= BRNF_COPY_MAC_SADDR;
> + skb_copy_from_linear_data_offset(oldskb, daddr_offset,
> + nskb->nf_bridge->data, 6);
Also proper ETH_* values please. But I'm wondering, we already save
the entire header in br_nf_post_routing(). Can't that be done earlier
so the upper layers don't have to care about this stuff and can simply
attach the original nf_bridge reference?
I'm also wondering - how are ICMP rejects handled?
> + } else
> #endif
> - )
> + if (hook != NF_INET_FORWARD)
> addr_type = RTN_LOCAL;
We used to route all bridged packets as RTN_LOCAL for some reason
which I'm not sure of. This is not necessary anymore?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic
2009-11-05 14:32 ` Patrick McHardy
@ 2009-11-05 19:19 ` Bart De Schuymer
2009-11-06 16:03 ` Patrick McHardy
0 siblings, 1 reply; 12+ messages in thread
From: Bart De Schuymer @ 2009-11-05 19:19 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Netfilter Developer Mailing List
[-- Attachment #1: Type: text/plain, Size: 3996 bytes --]
Patrick McHardy schreef:
> Bart De Schuymer wrote:
>
>> Hi,
>>
>> The attached patch does the following:
>> 1. fix a bug introduced in commit
>> 9d02002d2dc2c7423e5891b97727fde4d667adf1 (2/10/2006) which made
>> ipt_REJECT stop work for bridged traffic (use of nskb instead of oldskb)
>> 2. use the correct source MAC address for the response (bug reported in
>> bug 531 of netfilter's bugzilla)
>>
>> Tested for plain IP traffic and IP traffic encapsulated inside a VLAN
>> header (should also work for PPPoE encapsulated IP traffic).
>>
>>
>> --- linux-2.6.31-uml/net/bridge/br_netfilter.c.fixed 2009-11-02 21:22:00.000000000 +0100
>> +++ linux-2.6.31-uml/net/bridge/br_netfilter.c 2009-11-03 22:18:41.000000000 +0100
>> @@ -775,6 +766,13 @@ static unsigned int br_nf_local_out(unsi
>> return NF_DROP;
>>
>> nf_bridge = skb->nf_bridge;
>> + /* Enable complete transparency for e.g. ipt_REJECT */
>> + if (nf_bridge->mask & BRNF_COPY_MAC_SADDR) {
>> + skb_copy_to_linear_data_offset(skb, -8, nf_bridge->data, 6);
>>
>
> Please use the proper ETH_*LEN values. I guess that would be
> skb_copy_to_linear_data_offset(skb, -(ETH_HLEN - ETH_ALEN),
> nf_bridge->data, ETH_ALEN)
>
>
Done, see attached patch.
>> + nf_bridge_put(nf_bridge);
>> + skb->nf_bridge = NULL;
>> + return NF_ACCEPT;
>>
>
> Shouldn't packets with BRNF_BRIDGED_DNAT continue through NF_BR_FORWARD
> like they used to?
>
>
Yes. It is impossible for an skbuff to have both flags set.
>> + }
>> if (!(nf_bridge->mask & BRNF_BRIDGED_DNAT))
>> return NF_ACCEPT;
>>
>> --- linux-2.6.31-uml/net/ipv4/netfilter/ipt_REJECT.c.ori 2009-10-31 19:31:54.000000000 +0100
>> +++ linux-2.6.31-uml/net/ipv4/netfilter/ipt_REJECT.c 2009-11-03 21:55:08.000000000 +0100
>> @@ -100,11 +100,19 @@ static void send_reset(struct sk_buff *o
>> sizeof(struct tcphdr), 0));
>>
>> addr_type = RTN_UNSPEC;
>> - if (hook != NF_INET_FORWARD
>> #ifdef CONFIG_BRIDGE_NETFILTER
>> - || (nskb->nf_bridge && nskb->nf_bridge->mask & BRNF_BRIDGED)
>> + if (oldskb->nf_bridge && oldskb->nf_bridge->mask & BRNF_BRIDGED) {
>> + int daddr_offset = -14 - nf_bridge_encap_header_len(oldskb);
>> +
>> + addr_type = RTN_LOCAL;
>> + if (!nf_bridge_alloc(nskb))
>> + goto free_nskb;
>> + nskb->nf_bridge->mask |= BRNF_COPY_MAC_SADDR;
>> + skb_copy_from_linear_data_offset(oldskb, daddr_offset,
>> + nskb->nf_bridge->data, 6);
>>
>
> Also proper ETH_* values please. But I'm wondering, we already save
> the entire header in br_nf_post_routing(). Can't that be done earlier
> so the upper layers don't have to care about this stuff and can simply
> attach the original nf_bridge reference?
>
>
If you don't save the correct MAC address for the newly created skbuff
in ipt_REJECT, there is no way to get it back later. Furthermore, if you
save the header too early, MAC SNAT and DNAT might have changed the
header and you have to resave the header anyway.
> I'm also wondering - how are ICMP rejects handled?
>
>
Good question :-)
ICMP packets currently get sent with a source IP and MAC address of the
bridge. If the bridge doesn't have an IP address but does have a
suitable route, the source address is 0.0.0.0. I'll look into fixing this.
>> + } else
>> #endif
>> - )
>> + if (hook != NF_INET_FORWARD)
>> addr_type = RTN_LOCAL;
>>
>
> We used to route all bridged packets as RTN_LOCAL for some reason
> which I'm not sure of. This is not necessary anymore?
>
>
Yes it is still necessary and it's in the patch. There used to be
special code for bridge-netfilter in send_reset because we didn't want
to enforce ip_forward to be enabled to be able to send the reset answers
(there used to be a comment about that in ipt_REJECT). However, the
commit I mentioned above rewrote the send_reset function and enabling
ip_forward isn't enough anymore: it doesn't work without RTN_LOCAL.
cheers,
Bart
--
Bart De Schuymer
www.artinalgorithms.be
[-- Attachment #2: ipt_REJECT_use_correct_SMAC.diff --]
[-- Type: text/plain, Size: 2867 bytes --]
--- linux-2.6.31-uml/include/linux/netfilter_bridge.h.ori 2009-11-02 20:58:59.000000000 +0100
+++ linux-2.6.31-uml/include/linux/netfilter_bridge.h 2009-11-02 19:58:09.000000000 +0100
@@ -44,6 +44,7 @@ enum nf_br_hook_priorities {
#define BRNF_DONT_TAKE_PARENT 0x04
#define BRNF_BRIDGED 0x08
#define BRNF_NF_BRIDGE_PREROUTING 0x10
+#define BRNF_COPY_MAC_SADDR 0x20
/* Only used in br_forward.c */
@@ -77,6 +78,15 @@ static inline unsigned int nf_bridge_pad
return 0;
}
+static inline struct nf_bridge_info *nf_bridge_alloc(struct sk_buff *skb)
+{
+ skb->nf_bridge = kzalloc(sizeof(struct nf_bridge_info), GFP_ATOMIC);
+ if (likely(skb->nf_bridge))
+ atomic_set(&(skb->nf_bridge->use), 1);
+
+ return skb->nf_bridge;
+}
+
struct bridge_skb_cb {
union {
__be32 ipv4;
--- linux-2.6.31-uml/net/bridge/br_netfilter.c.fixed 2009-11-02 21:22:00.000000000 +0100
+++ linux-2.6.31-uml/net/bridge/br_netfilter.c 2009-11-05 19:30:17.000000000 +0100
@@ -145,15 +145,6 @@ static inline struct net_device *bridge_
return port ? port->br->dev : NULL;
}
-static inline struct nf_bridge_info *nf_bridge_alloc(struct sk_buff *skb)
-{
- skb->nf_bridge = kzalloc(sizeof(struct nf_bridge_info), GFP_ATOMIC);
- if (likely(skb->nf_bridge))
- atomic_set(&(skb->nf_bridge->use), 1);
-
- return skb->nf_bridge;
-}
-
static inline struct nf_bridge_info *nf_bridge_unshare(struct sk_buff *skb)
{
struct nf_bridge_info *nf_bridge = skb->nf_bridge;
@@ -775,6 +766,14 @@ static unsigned int br_nf_local_out(unsi
return NF_DROP;
nf_bridge = skb->nf_bridge;
+ /* Enable complete transparency for e.g. ipt_REJECT */
+ if (nf_bridge->mask & BRNF_COPY_MAC_SADDR) {
+ skb_copy_to_linear_data_offset(skb, -(ETH_HLEN - ETH_ALEN),
+ nf_bridge->data, ETH_ALEN);
+ nf_bridge_put(nf_bridge);
+ skb->nf_bridge = NULL;
+ return NF_ACCEPT;
+ }
if (!(nf_bridge->mask & BRNF_BRIDGED_DNAT))
return NF_ACCEPT;
--- linux-2.6.31-uml/net/ipv4/netfilter/ipt_REJECT.c.ori 2009-10-31 19:31:54.000000000 +0100
+++ linux-2.6.31-uml/net/ipv4/netfilter/ipt_REJECT.c 2009-11-05 19:29:58.000000000 +0100
@@ -100,11 +100,19 @@ static void send_reset(struct sk_buff *o
sizeof(struct tcphdr), 0));
addr_type = RTN_UNSPEC;
- if (hook != NF_INET_FORWARD
#ifdef CONFIG_BRIDGE_NETFILTER
- || (nskb->nf_bridge && nskb->nf_bridge->mask & BRNF_BRIDGED)
+ if (oldskb->nf_bridge && oldskb->nf_bridge->mask & BRNF_BRIDGED) {
+ int daddr_offset = -ETH_HLEN - nf_bridge_encap_header_len(oldskb);
+
+ addr_type = RTN_LOCAL;
+ if (!nf_bridge_alloc(nskb))
+ goto free_nskb;
+ nskb->nf_bridge->mask |= BRNF_COPY_MAC_SADDR;
+ skb_copy_from_linear_data_offset(oldskb, daddr_offset,
+ nskb->nf_bridge->data, ETH_ALEN);
+ } else
#endif
- )
+ if (hook != NF_INET_FORWARD)
addr_type = RTN_LOCAL;
/* ip_route_me_harder expects skb->dst to be set */
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic
2009-11-05 19:19 ` Bart De Schuymer
@ 2009-11-06 16:03 ` Patrick McHardy
2009-11-06 17:33 ` Bart De Schuymer
0 siblings, 1 reply; 12+ messages in thread
From: Patrick McHardy @ 2009-11-06 16:03 UTC (permalink / raw)
To: Bart De Schuymer; +Cc: Netfilter Developer Mailing List
Bart De Schuymer wrote:
> Patrick McHardy schreef:
>> Also proper ETH_* values please. But I'm wondering, we already save
>> the entire header in br_nf_post_routing(). Can't that be done earlier
>> so the upper layers don't have to care about this stuff and can simply
>> attach the original nf_bridge reference?
>>
>>
> If you don't save the correct MAC address for the newly created skbuff
> in ipt_REJECT, there is no way to get it back later. Furthermore, if you
> save the header too early, MAC SNAT and DNAT might have changed the
> header and you have to resave the header anyway.
Yes, we need to save it at some point. My idea was that we might be able
to save it in PREROUTING instead of POSTROUTING and only do
nskb->nf_bridge = nf_bridge_get(oskb->nf_bridge)
in ipt_REJECT and probably also the ICMP code. MAC NAT could be handled
by updating the bridge info simultaneously.
>> I'm also wondering - how are ICMP rejects handled?
>>
>>
> Good question :-)
> ICMP packets currently get sent with a source IP and MAC address of the
> bridge. If the bridge doesn't have an IP address but does have a
> suitable route, the source address is 0.0.0.0. I'll look into fixing this.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic
2009-11-06 16:03 ` Patrick McHardy
@ 2009-11-06 17:33 ` Bart De Schuymer
2009-11-06 17:36 ` Patrick McHardy
0 siblings, 1 reply; 12+ messages in thread
From: Bart De Schuymer @ 2009-11-06 17:33 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Netfilter Developer Mailing List
Patrick McHardy schreef:
> Bart De Schuymer wrote:
>
>> Patrick McHardy schreef:
>>
>>> Also proper ETH_* values please. But I'm wondering, we already save
>>> the entire header in br_nf_post_routing(). Can't that be done earlier
>>> so the upper layers don't have to care about this stuff and can simply
>>> attach the original nf_bridge reference?
>>>
>>>
>>>
>> If you don't save the correct MAC address for the newly created skbuff
>> in ipt_REJECT, there is no way to get it back later. Furthermore, if you
>> save the header too early, MAC SNAT and DNAT might have changed the
>> header and you have to resave the header anyway.
>>
>
> Yes, we need to save it at some point. My idea was that we might be able
> to save it in PREROUTING instead of POSTROUTING and only do
>
> nskb->nf_bridge = nf_bridge_get(oskb->nf_bridge)
>
> in ipt_REJECT and probably also the ICMP code. MAC NAT could be handled
> by updating the bridge info simultaneously.
>
>
Patrick,
The code creates a new skbuf and the correct source MAC address is lost
if you don't attach it to the skbuf at that time. How will you know in
PREROUTING what SMAC to use if you didn't save it when you created the
skbuf?
cheers,
Bart
--
Bart De Schuymer
www.artinalgorithms.be
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic
2009-11-06 17:33 ` Bart De Schuymer
@ 2009-11-06 17:36 ` Patrick McHardy
2009-11-06 17:45 ` Patrick McHardy
2009-11-06 17:46 ` Patrick McHardy
0 siblings, 2 replies; 12+ messages in thread
From: Patrick McHardy @ 2009-11-06 17:36 UTC (permalink / raw)
To: Bart De Schuymer; +Cc: Netfilter Developer Mailing List
Bart De Schuymer wrote:
> Patrick McHardy schreef:
>> Yes, we need to save it at some point. My idea was that we might be able
>> to save it in PREROUTING instead of POSTROUTING and only do
>>
>> nskb->nf_bridge = nf_bridge_get(oskb->nf_bridge)
>>
>> in ipt_REJECT and probably also the ICMP code. MAC NAT could be handled
>> by updating the bridge info simultaneously.
>>
>>
> The code creates a new skbuf and the correct source MAC address is lost
> if you don't attach it to the skbuf at that time.
That's what I'm doing above.
> How will you know in
> PREROUTING what SMAC to use if you didn't save it when you created the
> skbuf?
I'm not sure I understand what you're getting at. The above
line of code would do exactly that, attach the nf_bridge
data from the original packet to the newly created one.
But for this to work we need to make sure its valid in all
hooks, hence my suggestion to save it in PREROUTING instead
of POSTROUTING.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic
2009-11-06 17:36 ` Patrick McHardy
@ 2009-11-06 17:45 ` Patrick McHardy
2009-11-06 17:46 ` Patrick McHardy
2009-11-06 17:46 ` Patrick McHardy
1 sibling, 1 reply; 12+ messages in thread
From: Patrick McHardy @ 2009-11-06 17:45 UTC (permalink / raw)
To: Bart De Schuymer; +Cc: Netfilter Developer Mailing List
Patrick McHardy wrote:
> Bart De Schuymer wrote:
>> Patrick McHardy schreef:
>>> Yes, we need to save it at some point. My idea was that we might be able
>>> to save it in PREROUTING instead of POSTROUTING and only do
>>>
>>> nskb->nf_bridge = nf_bridge_get(oskb->nf_bridge)
>>>
>>> in ipt_REJECT and probably also the ICMP code. MAC NAT could be handled
>>> by updating the bridge info simultaneously.
>>>
>>>
>> The code creates a new skbuf and the correct source MAC address is lost
>> if you don't attach it to the skbuf at that time.
>
> That's what I'm doing above.
>
>> How will you know in
>> PREROUTING what SMAC to use if you didn't save it when you created the
>> skbuf?
>
> I'm not sure I understand what you're getting at. The above
> line of code would do exactly that, attach the nf_bridge
> data from the original packet to the newly created one.
> But for this to work we need to make sure its valid in all
> hooks, hence my suggestion to save it in PREROUTING instead
> of POSTROUTING.
This patchTo demonstrate the idea
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic
2009-11-06 17:45 ` Patrick McHardy
@ 2009-11-06 17:46 ` Patrick McHardy
0 siblings, 0 replies; 12+ messages in thread
From: Patrick McHardy @ 2009-11-06 17:46 UTC (permalink / raw)
To: Bart De Schuymer; +Cc: Netfilter Developer Mailing List
Patrick McHardy wrote:
> Patrick McHardy wrote:
>> Bart De Schuymer wrote:
>>> Patrick McHardy schreef:
>>>> Yes, we need to save it at some point. My idea was that we might be able
>>>> to save it in PREROUTING instead of POSTROUTING and only do
>>>>
>>>> nskb->nf_bridge = nf_bridge_get(oskb->nf_bridge)
>>>>
>>>> in ipt_REJECT and probably also the ICMP code. MAC NAT could be handled
>>>> by updating the bridge info simultaneously.
>>>>
>>>>
>>> The code creates a new skbuf and the correct source MAC address is lost
>>> if you don't attach it to the skbuf at that time.
>> That's what I'm doing above.
>>
>>> How will you know in
>>> PREROUTING what SMAC to use if you didn't save it when you created the
>>> skbuf?
>> I'm not sure I understand what you're getting at. The above
>> line of code would do exactly that, attach the nf_bridge
>> data from the original packet to the newly created one.
>> But for this to work we need to make sure its valid in all
>> hooks, hence my suggestion to save it in PREROUTING instead
>> of POSTROUTING.
>
> This patchTo demonstrate the idea
Please ignore, hit the wrong key :)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic
2009-11-06 17:36 ` Patrick McHardy
2009-11-06 17:45 ` Patrick McHardy
@ 2009-11-06 17:46 ` Patrick McHardy
2009-11-06 18:21 ` Bart De Schuymer
2009-11-06 19:51 ` Jan Engelhardt
1 sibling, 2 replies; 12+ messages in thread
From: Patrick McHardy @ 2009-11-06 17:46 UTC (permalink / raw)
To: Bart De Schuymer; +Cc: Netfilter Developer Mailing List
[-- Attachment #1: Type: text/plain, Size: 1129 bytes --]
Patrick McHardy wrote:
> Bart De Schuymer wrote:
>> Patrick McHardy schreef:
>>> Yes, we need to save it at some point. My idea was that we might be able
>>> to save it in PREROUTING instead of POSTROUTING and only do
>>>
>>> nskb->nf_bridge = nf_bridge_get(oskb->nf_bridge)
>>>
>>> in ipt_REJECT and probably also the ICMP code. MAC NAT could be handled
>>> by updating the bridge info simultaneously.
>>>
>>>
>> The code creates a new skbuf and the correct source MAC address is lost
>> if you don't attach it to the skbuf at that time.
>
> That's what I'm doing above.
>
>> How will you know in
>> PREROUTING what SMAC to use if you didn't save it when you created the
>> skbuf?
>
> I'm not sure I understand what you're getting at. The above
> line of code would do exactly that, attach the nf_bridge
> data from the original packet to the newly created one.
> But for this to work we need to make sure its valid in all
> hooks, hence my suggestion to save it in PREROUTING instead
> of POSTROUTING.
This patch demonstrates the idea. Its not compile tested
and incomplete, just to make more clear what I'm suggesting.
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 2970 bytes --]
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 0c68fbd..410b0dc 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1952,10 +1952,11 @@ static inline void nf_bridge_put(struct nf_bridge_info *nf_bridge)
if (nf_bridge && atomic_dec_and_test(&nf_bridge->use))
kfree(nf_bridge);
}
-static inline void nf_bridge_get(struct nf_bridge_info *nf_bridge)
+static inline struct nf_bridge_info *nf_bridge_get(struct nf_bridge_info *nf_bridge)
{
if (nf_bridge)
atomic_inc(&nf_bridge->use);
+ return nf_bridge;
}
#endif /* CONFIG_BRIDGE_NETFILTER */
static inline void nf_reset(struct sk_buff *skb)
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index a16a234..0732b3b 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -567,6 +567,8 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff *skb,
return NF_ACCEPT;
#endif
nf_bridge_pull_encap_header_rcsum(skb);
+ nf_bridge_save_header(skb);
+
return br_nf_pre_routing_ipv6(hook, skb, in, out, okfn);
}
#ifdef CONFIG_SYSCTL
@@ -579,6 +581,7 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff *skb,
return NF_ACCEPT;
nf_bridge_pull_encap_header_rcsum(skb);
+ nf_bridge_save_header(skb);
if (!pskb_may_pull(skb, sizeof(struct iphdr)))
goto inhdr_error;
@@ -863,7 +866,6 @@ static unsigned int br_nf_post_routing(unsigned int hook, struct sk_buff *skb,
}
nf_bridge_pull_encap_header(skb);
- nf_bridge_save_header(skb);
NF_HOOK(pf, NF_INET_POST_ROUTING, skb, NULL, realoutdev,
br_nf_dev_queue_xmit);
diff --git a/net/bridge/netfilter/ebt_dnat.c b/net/bridge/netfilter/ebt_dnat.c
index 6b49ea9..3f4acfa 100644
--- a/net/bridge/netfilter/ebt_dnat.c
+++ b/net/bridge/netfilter/ebt_dnat.c
@@ -23,6 +23,7 @@ ebt_dnat_tg(struct sk_buff *skb, const struct xt_target_param *par)
return EBT_DROP;
memcpy(eth_hdr(skb)->h_dest, info->mac, ETH_ALEN);
+ /* update nf_bridge info */
return info->target;
}
diff --git a/net/bridge/netfilter/ebt_snat.c b/net/bridge/netfilter/ebt_snat.c
index 8d04d4c..edd4682 100644
--- a/net/bridge/netfilter/ebt_snat.c
+++ b/net/bridge/netfilter/ebt_snat.c
@@ -25,6 +25,7 @@ ebt_snat_tg(struct sk_buff *skb, const struct xt_target_param *par)
return EBT_DROP;
memcpy(eth_hdr(skb)->h_source, info->mac, ETH_ALEN);
+ /* update nf_bridge info */
if (!(info->target & NAT_ARP_BIT) &&
eth_hdr(skb)->h_proto == htons(ETH_P_ARP)) {
const struct arphdr *ap;
diff --git a/net/ipv4/netfilter/ipt_REJECT.c b/net/ipv4/netfilter/ipt_REJECT.c
index c93ae44..f66a7cc 100644
--- a/net/ipv4/netfilter/ipt_REJECT.c
+++ b/net/ipv4/netfilter/ipt_REJECT.c
@@ -107,6 +107,8 @@ static void send_reset(struct sk_buff *oldskb, int hook)
)
addr_type = RTN_LOCAL;
+ nskb->nf_bridge = nf_bridge_get(oldskb->nf_bridge);
+
/* ip_route_me_harder expects skb->dst to be set */
skb_dst_set(nskb, dst_clone(skb_dst(oldskb)));
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic
2009-11-06 17:46 ` Patrick McHardy
@ 2009-11-06 18:21 ` Bart De Schuymer
2009-11-06 18:30 ` Patrick McHardy
2009-11-06 19:51 ` Jan Engelhardt
1 sibling, 1 reply; 12+ messages in thread
From: Bart De Schuymer @ 2009-11-06 18:21 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Netfilter Developer Mailing List
Patrick McHardy schreef:
> Patrick McHardy wrote:
>
>> Bart De Schuymer wrote:
>>
>>> Patrick McHardy schreef:
>>>
>>>> Yes, we need to save it at some point. My idea was that we might be able
>>>> to save it in PREROUTING instead of POSTROUTING and only do
>>>>
>>>> nskb->nf_bridge = nf_bridge_get(oskb->nf_bridge)
>>>>
>>>> in ipt_REJECT and probably also the ICMP code. MAC NAT could be handled
>>>> by updating the bridge info simultaneously.
>>>>
>>>>
>>>>
>>> The code creates a new skbuf and the correct source MAC address is lost
>>> if you don't attach it to the skbuf at that time.
>>>
>> That's what I'm doing above.
>>
>>
>>> How will you know in
>>> PREROUTING what SMAC to use if you didn't save it when you created the
>>> skbuf?
>>>
>> I'm not sure I understand what you're getting at. The above
>> line of code would do exactly that, attach the nf_bridge
>> data from the original packet to the newly created one.
>> But for this to work we need to make sure its valid in all
>> hooks, hence my suggestion to save it in PREROUTING instead
>> of POSTROUTING.
>>
>
> This patch demonstrates the idea. Its not compile tested
> and incomplete, just to make more clear what I'm suggesting.
>
OK, that sounds good. Much cleaner. Sorry for misunderstanding you, it's
Friday evening...
I'll look into it.
cheers,
Bart
--
Bart De Schuymer
www.artinalgorithms.be
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic
2009-11-06 18:21 ` Bart De Schuymer
@ 2009-11-06 18:30 ` Patrick McHardy
0 siblings, 0 replies; 12+ messages in thread
From: Patrick McHardy @ 2009-11-06 18:30 UTC (permalink / raw)
To: Bart De Schuymer; +Cc: Netfilter Developer Mailing List
Bart De Schuymer wrote:
> Patrick McHardy schreef:
>>
>> This patch demonstrates the idea. Its not compile tested
>> and incomplete, just to make more clear what I'm suggesting.
>>
> OK, that sounds good. Much cleaner. Sorry for misunderstanding you, it's
> Friday evening...
> I'll look into it.
Great, thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic
2009-11-06 17:46 ` Patrick McHardy
2009-11-06 18:21 ` Bart De Schuymer
@ 2009-11-06 19:51 ` Jan Engelhardt
1 sibling, 0 replies; 12+ messages in thread
From: Jan Engelhardt @ 2009-11-06 19:51 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Bart De Schuymer, Netfilter Developer Mailing List
On Friday 2009-11-06 18:46, Patrick McHardy wrote:
>>
>> I'm not sure I understand what you're getting at. The above
>> line of code would do exactly that, attach the nf_bridge
>> data from the original packet to the newly created one.
>> But for this to work we need to make sure its valid in all
>> hooks, hence my suggestion to save it in PREROUTING instead
>> of POSTROUTING.
>
>This patch demonstrates the idea. Its not compile tested
>and incomplete, just to make more clear what I'm suggesting.
>
[patch in non-inlined form]
For a final one you also want to modify ip6t_REJECT.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2009-11-06 19:51 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-04 19:05 [PATCH][BRIDGE-NETFILTER] fix REJECT for bridged traffic Bart De Schuymer
2009-11-05 14:32 ` Patrick McHardy
2009-11-05 19:19 ` Bart De Schuymer
2009-11-06 16:03 ` Patrick McHardy
2009-11-06 17:33 ` Bart De Schuymer
2009-11-06 17:36 ` Patrick McHardy
2009-11-06 17:45 ` Patrick McHardy
2009-11-06 17:46 ` Patrick McHardy
2009-11-06 17:46 ` Patrick McHardy
2009-11-06 18:21 ` Bart De Schuymer
2009-11-06 18:30 ` Patrick McHardy
2009-11-06 19:51 ` Jan Engelhardt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.