[Openvpn-devel,v5,1/3] pf: restyle pf_c2c/addr_test() to make them 'struct context' agnostic

  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(-)
  

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