| Message ID | 20171111161836.23356-1-a@unstable.cc |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,v5,1/3] pf: restyle pf_c2c/addr_test() to make them 'struct context' agnostic | expand |
Hi, On 11-11-17 17:18, Antonio Quartulli wrote: > In the attempt of getting rid of any pf-inline.h file, we need > to make sure that inline functions do not trigger any circular > include dependency. > > For this reason, avoid pf_c2c/addr_test() to be 'struct context' > aware, so that pf-inline.h does not need to rely on the content > of openvpn.h. > > Cc: Steffan Karger <steffan@karger.me> > Signed-off-by: Antonio Quartulli <a@unstable.cc> > --- > > v1-v3: skipped > v4: this is the first version of this patch, but named v4 for convenience > v5: follow Steffan's suggestion and make pf_c2c_test() take tls_multi as argument > > > src/openvpn/multi.c | 28 +++++++++++++++++++++------- > src/openvpn/pf-inline.h | 14 +++++++++----- > 2 files changed, 30 insertions(+), 12 deletions(-) > > diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c > index 82a0b9d9..5c2c8e69 100644 > --- a/src/openvpn/multi.c > +++ b/src/openvpn/multi.c > @@ -2230,7 +2230,11 @@ multi_bcast(struct multi_context *m, > #ifdef ENABLE_PF > if (sender_instance) > { > - if (!pf_c2c_test(&sender_instance->context, &mi->context, "bcast_c2c")) > + if (!pf_c2c_test(&sender_instance->context.c2.pf, > + sender_instance->context.c2.tls_multi, > + &mi->context.c2.pf, > + mi->context.c2.tls_multi, > + "bcast_c2c")) > { > msg(D_PF_DROPPED_BCAST, "PF: client[%s] -> client[%s] packet dropped by BCAST packet filter", > mi_prefix(sender_instance), > @@ -2240,7 +2244,8 @@ multi_bcast(struct multi_context *m, > } > if (sender_addr) > { > - if (!pf_addr_test(&mi->context, sender_addr, "bcast_src_addr")) > + if (!pf_addr_test(&mi->context.c2.pf, &mi->context, > + sender_addr, "bcast_src_addr")) > { > struct gc_arena gc = gc_new(); > msg(D_PF_DROPPED_BCAST, "PF: addr[%s] -> client[%s] packet dropped by BCAST packet filter", > @@ -2599,7 +2604,10 @@ multi_process_incoming_link(struct multi_context *m, struct multi_instance *inst > if (mi) > { > #ifdef ENABLE_PF > - if (!pf_c2c_test(c, &mi->context, "tun_c2c")) > + if (!pf_c2c_test(&c->c2.pf, c->c2.tls_multi, > + &mi->context.c2.pf, > + mi->context.c2.tls_multi, > + "tun_c2c")) > { > msg(D_PF_DROPPED, "PF: client -> client[%s] packet dropped by TUN packet filter", > mi_prefix(mi)); > @@ -2615,7 +2623,8 @@ multi_process_incoming_link(struct multi_context *m, struct multi_instance *inst > } > } > #ifdef ENABLE_PF > - if (c->c2.to_tun.len && !pf_addr_test(c, &dest, "tun_dest_addr")) > + if (c->c2.to_tun.len && !pf_addr_test(&c->c2.pf, c, &dest, > + "tun_dest_addr")) > { > msg(D_PF_DROPPED, "PF: client -> addr[%s] packet dropped by TUN packet filter", > mroute_addr_print_ex(&dest, MAPF_SHOW_ARP, &gc)); > @@ -2660,7 +2669,10 @@ multi_process_incoming_link(struct multi_context *m, struct multi_instance *inst > if (mi) > { > #ifdef ENABLE_PF > - if (!pf_c2c_test(c, &mi->context, "tap_c2c")) > + if (!pf_c2c_test(&c->c2.pf, c->c2.tls_multi, > + &mi->context.c2.pf, > + mi->context.c2.tls_multi, > + "tap_c2c")) > { > msg(D_PF_DROPPED, "PF: client -> client[%s] packet dropped by TAP packet filter", > mi_prefix(mi)); > @@ -2676,7 +2688,9 @@ multi_process_incoming_link(struct multi_context *m, struct multi_instance *inst > } > } > #ifdef ENABLE_PF > - if (c->c2.to_tun.len && !pf_addr_test(c, &edest, "tap_dest_addr")) > + if (c->c2.to_tun.len && !pf_addr_test(&c->c2.pf, c, > + &edest, > + "tap_dest_addr")) > { > msg(D_PF_DROPPED, "PF: client -> addr[%s] packet dropped by TAP packet filter", > mroute_addr_print_ex(&edest, MAPF_SHOW_ARP, &gc)); > @@ -2789,7 +2803,7 @@ multi_process_incoming_tun(struct multi_context *m, const unsigned int mpp_flags > set_prefix(m->pending); > > #ifdef ENABLE_PF > - if (!pf_addr_test(c, e2, "tun_tap_src_addr")) > + if (!pf_addr_test(&c->c2.pf, c, e2, "tun_tap_src_addr")) > { > msg(D_PF_DROPPED, "PF: addr[%s] -> client packet dropped by packet filter", > mroute_addr_print_ex(&src, MAPF_SHOW_ARP, &gc)); > diff --git a/src/openvpn/pf-inline.h b/src/openvpn/pf-inline.h > index ac19ac4c..3ba90ccf 100644 > --- a/src/openvpn/pf-inline.h > +++ b/src/openvpn/pf-inline.h > @@ -31,20 +31,24 @@ > #define PCT_SRC 1 > #define PCT_DEST 2 > static inline bool > -pf_c2c_test(const struct context *src, const struct context *dest, const char *prefix) > +pf_c2c_test(const struct pf_context *src_pf, const struct tls_multi *src, > + const struct pf_context *dest_pf, const struct tls_multi *dest, > + const char *prefix) > { > bool pf_cn_test(struct pf_set *pfs, const struct tls_multi *tm, const int type, const char *prefix); > > - return (!src->c2.pf.enabled || pf_cn_test(src->c2.pf.pfs, dest->c2.tls_multi, PCT_DEST, prefix)) > - && (!dest->c2.pf.enabled || pf_cn_test(dest->c2.pf.pfs, src->c2.tls_multi, PCT_SRC, prefix)); > + return (!src_pf->enabled || pf_cn_test(src_pf->pfs, dest, PCT_DEST, prefix)) > + && (!dest_pf->enabled || pf_cn_test(dest_pf->pfs, src, PCT_SRC, > + prefix)); > } > > static inline bool > -pf_addr_test(const struct context *src, const struct mroute_addr *dest, const char *prefix) > +pf_addr_test(const struct pf_context *src_pf, const struct context *src, > + const struct mroute_addr *dest, const char *prefix) > { > bool pf_addr_test_dowork(const struct context *src, const struct mroute_addr *dest, const char *prefix); > > - if (src->c2.pf.enabled) > + if (src_pf->enabled) > { > return pf_addr_test_dowork(src, dest, prefix); > } > Looks good to me now. Reviewed-by: Steffan Karger <steffan@karger.me> Acked-by: Steffan Karger <steffan@karger.me> -Steffan ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Your patch has been applied to the master branch.
commit 9646caeae3b3879e1d422405e42b7fbd05cb30a9
Author: Antonio Quartulli
Date: Sun Nov 12 00:18:34 2017 +0800
pf: restyle pf_c2c/addr_test() to make them 'struct context' agnostic
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20171111161836.23356-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15822.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 82a0b9d9..5c2c8e69 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2230,7 +2230,11 @@ multi_bcast(struct multi_context *m, #ifdef ENABLE_PF if (sender_instance) { - if (!pf_c2c_test(&sender_instance->context, &mi->context, "bcast_c2c")) + if (!pf_c2c_test(&sender_instance->context.c2.pf, + sender_instance->context.c2.tls_multi, + &mi->context.c2.pf, + mi->context.c2.tls_multi, + "bcast_c2c")) { msg(D_PF_DROPPED_BCAST, "PF: client[%s] -> client[%s] packet dropped by BCAST packet filter", mi_prefix(sender_instance), @@ -2240,7 +2244,8 @@ multi_bcast(struct multi_context *m, } if (sender_addr) { - if (!pf_addr_test(&mi->context, sender_addr, "bcast_src_addr")) + if (!pf_addr_test(&mi->context.c2.pf, &mi->context, + sender_addr, "bcast_src_addr")) { struct gc_arena gc = gc_new(); msg(D_PF_DROPPED_BCAST, "PF: addr[%s] -> client[%s] packet dropped by BCAST packet filter", @@ -2599,7 +2604,10 @@ multi_process_incoming_link(struct multi_context *m, struct multi_instance *inst if (mi) { #ifdef ENABLE_PF - if (!pf_c2c_test(c, &mi->context, "tun_c2c")) + if (!pf_c2c_test(&c->c2.pf, c->c2.tls_multi, + &mi->context.c2.pf, + mi->context.c2.tls_multi, + "tun_c2c")) { msg(D_PF_DROPPED, "PF: client -> client[%s] packet dropped by TUN packet filter", mi_prefix(mi)); @@ -2615,7 +2623,8 @@ multi_process_incoming_link(struct multi_context *m, struct multi_instance *inst } } #ifdef ENABLE_PF - if (c->c2.to_tun.len && !pf_addr_test(c, &dest, "tun_dest_addr")) + if (c->c2.to_tun.len && !pf_addr_test(&c->c2.pf, c, &dest, + "tun_dest_addr")) { msg(D_PF_DROPPED, "PF: client -> addr[%s] packet dropped by TUN packet filter", mroute_addr_print_ex(&dest, MAPF_SHOW_ARP, &gc)); @@ -2660,7 +2669,10 @@ multi_process_incoming_link(struct multi_context *m, struct multi_instance *inst if (mi) { #ifdef ENABLE_PF - if (!pf_c2c_test(c, &mi->context, "tap_c2c")) + if (!pf_c2c_test(&c->c2.pf, c->c2.tls_multi, + &mi->context.c2.pf, + mi->context.c2.tls_multi, + "tap_c2c")) { msg(D_PF_DROPPED, "PF: client -> client[%s] packet dropped by TAP packet filter", mi_prefix(mi)); @@ -2676,7 +2688,9 @@ multi_process_incoming_link(struct multi_context *m, struct multi_instance *inst } } #ifdef ENABLE_PF - if (c->c2.to_tun.len && !pf_addr_test(c, &edest, "tap_dest_addr")) + if (c->c2.to_tun.len && !pf_addr_test(&c->c2.pf, c, + &edest, + "tap_dest_addr")) { msg(D_PF_DROPPED, "PF: client -> addr[%s] packet dropped by TAP packet filter", mroute_addr_print_ex(&edest, MAPF_SHOW_ARP, &gc)); @@ -2789,7 +2803,7 @@ multi_process_incoming_tun(struct multi_context *m, const unsigned int mpp_flags set_prefix(m->pending); #ifdef ENABLE_PF - if (!pf_addr_test(c, e2, "tun_tap_src_addr")) + if (!pf_addr_test(&c->c2.pf, c, e2, "tun_tap_src_addr")) { msg(D_PF_DROPPED, "PF: addr[%s] -> client packet dropped by packet filter", mroute_addr_print_ex(&src, MAPF_SHOW_ARP, &gc)); diff --git a/src/openvpn/pf-inline.h b/src/openvpn/pf-inline.h index ac19ac4c..3ba90ccf 100644 --- a/src/openvpn/pf-inline.h +++ b/src/openvpn/pf-inline.h @@ -31,20 +31,24 @@ #define PCT_SRC 1 #define PCT_DEST 2 static inline bool -pf_c2c_test(const struct context *src, const struct context *dest, const char *prefix) +pf_c2c_test(const struct pf_context *src_pf, const struct tls_multi *src, + const struct pf_context *dest_pf, const struct tls_multi *dest, + const char *prefix) { bool pf_cn_test(struct pf_set *pfs, const struct tls_multi *tm, const int type, const char *prefix); - return (!src->c2.pf.enabled || pf_cn_test(src->c2.pf.pfs, dest->c2.tls_multi, PCT_DEST, prefix)) - && (!dest->c2.pf.enabled || pf_cn_test(dest->c2.pf.pfs, src->c2.tls_multi, PCT_SRC, prefix)); + return (!src_pf->enabled || pf_cn_test(src_pf->pfs, dest, PCT_DEST, prefix)) + && (!dest_pf->enabled || pf_cn_test(dest_pf->pfs, src, PCT_SRC, + prefix)); } static inline bool -pf_addr_test(const struct context *src, const struct mroute_addr *dest, const char *prefix) +pf_addr_test(const struct pf_context *src_pf, const struct context *src, + const struct mroute_addr *dest, const char *prefix) { bool pf_addr_test_dowork(const struct context *src, const struct mroute_addr *dest, const char *prefix); - if (src->c2.pf.enabled) + if (src_pf->enabled) { return pf_addr_test_dowork(src, dest, prefix); }
In the attempt of getting rid of any pf-inline.h file, we need to make sure that inline functions do not trigger any circular include dependency. For this reason, avoid pf_c2c/addr_test() to be 'struct context' aware, so that pf-inline.h does not need to rely on the content of openvpn.h. Cc: Steffan Karger <steffan@karger.me> Signed-off-by: Antonio Quartulli <a@unstable.cc> --- v1-v3: skipped v4: this is the first version of this patch, but named v4 for convenience v5: follow Steffan's suggestion and make pf_c2c_test() take tls_multi as argument src/openvpn/multi.c | 28 +++++++++++++++++++++------- src/openvpn/pf-inline.h | 14 +++++++++----- 2 files changed, 30 insertions(+), 12 deletions(-)