diff mbox

[OpenWrt-Devel] set mark for locally generated traffic in OUTPUT chain

Message ID 20160428122458.GA11150@makrotopia.org
State Accepted
Delegated to: Jo-Philipp Wich
Headers show

Commit Message

Daniel Golle April 28, 2016, 12:25 p.m. UTC
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 rules.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

John Crispin April 28, 2016, 12:57 p.m. UTC | #1
trivia is missing



On 28/04/2016 14:25, Daniel Golle wrote:
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  rules.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/rules.c b/rules.c
> index 756c78d..e20442e 100644
> --- a/rules.c
> +++ b/rules.c
> @@ -270,7 +270,7 @@ append_chain(struct fw3_ipt_rule *r, struct fw3_rule *rule)
>  	{
>  		snprintf(chain, sizeof(chain), "zone_%s_notrack", rule->src.name);
>  	}
> -	else if (rule->target == FW3_FLAG_MARK)
> +	else if (rule->target == FW3_FLAG_MARK && (rule->_src || rule->src.any))
>  	{
>  		snprintf(chain, sizeof(chain), "PREROUTING");
>  	}
>
Jo-Philipp Wich April 28, 2016, 1:03 p.m. UTC | #2
Hi Daniel,

my stomach feeling tells me that this is okay but can you elaborate on
the change?

~ Jo
Daniel Golle April 28, 2016, 1:31 p.m. UTC | #3
Hi Jo!

On Thu, Apr 28, 2016 at 03:03:49PM +0200, Jo-Philipp Wich wrote:
> my stomach feeling tells me that this is okay but can you elaborate on
> the change?

Currently all marking always happends in the PREROUTING chain. This
doesn't catch locally generated traffic and thus also doesn't allow
using match extensions like cgroup, owner or socket which are useful
e.g. to re-route traffic from a specific local process (eg. dnsmasq).
Similar to the rules in other tables, I'd assume that rules without any
source should affect locally generated traffic and thus MARK rules
should end up in the mangle table's OUTPUT chain in that case.


Cheers


Daniel
Hans Dedecker April 28, 2016, 1:57 p.m. UTC | #4
On Thu, Apr 28, 2016 at 3:31 PM, Daniel Golle <daniel@makrotopia.org> wrote:

> Hi Jo!
>
> On Thu, Apr 28, 2016 at 03:03:49PM +0200, Jo-Philipp Wich wrote:
> > my stomach feeling tells me that this is okay but can you elaborate on
> > the change?
>
> Currently all marking always happends in the PREROUTING chain. This
> doesn't catch locally generated traffic and thus also doesn't allow
> using match extensions like cgroup, owner or socket which are useful
> e.g. to re-route traffic from a specific local process (eg. dnsmasq).
> Similar to the rules in other tables, I'd assume that rules without any
> source should affect locally generated traffic and thus MARK rules
> should end up in the mangle table's OUTPUT chain in that case.
>
Indeed an extension which is very much appreciated as we have similar use
cases where we mark local traffic for routing decisions, etc ...

Hans

>
>
> Cheers
>
>
> Daniel
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>
Jo-Philipp Wich April 28, 2016, 2:03 p.m. UTC | #5
Hi Daniel,

thanks for the explaination, I was missing the function context.

The change is indeed fine, pushed to firewall3.git - thanks!

~ Jo
diff mbox

Patch

diff --git a/rules.c b/rules.c
index 756c78d..e20442e 100644
--- a/rules.c
+++ b/rules.c
@@ -270,7 +270,7 @@  append_chain(struct fw3_ipt_rule *r, struct fw3_rule *rule)
 	{
 		snprintf(chain, sizeof(chain), "zone_%s_notrack", rule->src.name);
 	}
-	else if (rule->target == FW3_FLAG_MARK)
+	else if (rule->target == FW3_FLAG_MARK && (rule->_src || rule->src.any))
 	{
 		snprintf(chain, sizeof(chain), "PREROUTING");
 	}