Hello,
I have just nit comments, feel free to ignore them.
1) would it be possible to use closing #endif guards as follows:
#endif /* NCARP */
2) another nit here:
> @@ -5607,6 +5616,8 @@ pf_route(struct mbuf **m, struct pf_rule
> done:
> if (r->rt != PF_DUPTO)
> *m = NULL;
> + if (!r->rt)
> + if_put(ifp);
> rtfree(rt);
> return;
>
I would probably use test as follows:
if (rt != NULL)
if_put(ifp);
it's detail, given at some point in future we will probably have to use
if_put()/if_get() for ifp's bound to kif's (right?). My point here is we call
if_get() after we successfully obtain rt.
Apart those two everything is OK, since those two are nits, it's up to you if
you go for my suggestions.
regards
sasha
On Thu, Nov 19, 2015 at 12:07:38PM +0100, Martin Pieuchot wrote:
> Stop using rt_ifp. While here put some NCARP... ok?
>
> Index: net/pf.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.950
> diff -u -p -r1.950 pf.c
> --- net/pf.c 12 Nov 2015 10:07:14 -0000 1.950
> +++ net/pf.c 19 Nov 2015 11:05:37 -0000
> @@ -36,6 +36,7 @@
> */
>
> #include "bpfilter.h"
> +#include "carp.h"
> #include "pflog.h"
> #include "pfsync.h"
> #include "pflow.h"
> @@ -2595,9 +2596,11 @@ pf_match_rcvif(struct mbuf *m, struct pf
> if (ifp == NULL)
> return (0);
>
> +#if NCARP > 0
> if (ifp->if_type == IFT_CARP && ifp->if_carpdev)
> kif = (struct pfi_kif *)ifp->if_carpdev->if_pf_kif;
> else
> +#endif
> kif = (struct pfi_kif *)ifp->if_pf_kif;
>
> if_put(ifp);
> @@ -5347,7 +5350,6 @@ pf_routable(struct pf_addr *addr, sa_fam
> struct sockaddr_in6 *dst6;
> #endif /* INET6 */
> struct rtentry *rt, *rt0 = NULL;
> - struct ifnet *ifp;
>
> check_mpath = 0;
> memset(&ss, 0, sizeof(ss));
> @@ -5397,13 +5399,20 @@ pf_routable(struct pf_addr *addr, sa_fam
> ret = 0;
> rt = rt0;
> do {
> - if (rt->rt_ifp->if_type == IFT_CARP)
> - ifp = rt->rt_ifp->if_carpdev;
> - else
> - ifp = rt->rt_ifp;
> -
> - if (kif->pfik_ifp == ifp)
> + if (rt->rt_ifidx == kif->pfik_ifp->if_index) {
> ret = 1;
> +#if NCARP > 0
> + } else {
> + struct ifnet *ifp;
> +
> + ifp = if_get(rt->rt_ifidx);
> + if (ifp != NULL && ifp->if_type == IFT_CARP &&
> + ifp->if_carpdev == kif->pfik_ifp)
> + ret = 1;
> + if_put(ifp);
> +#endif
> + }
> +
> #ifndef SMALL_KERNEL
> rt = rtable_mpath_next(rt);
> #else
> @@ -5512,7 +5521,7 @@ pf_route(struct mbuf **m, struct pf_rule
> goto bad;
> }
>
> - ifp = rt->rt_ifp;
> + ifp = if_get(rt->rt_ifidx);
>
> if (rt->rt_flags & RTF_GATEWAY)
> dst = satosin(rt->rt_gateway);
> @@ -5607,6 +5616,8 @@ pf_route(struct mbuf **m, struct pf_rule
> done:
> if (r->rt != PF_DUPTO)
> *m = NULL;
> + if (!r->rt)
> + if_put(ifp);
> rtfree(rt);
> return;
>
> @@ -6312,9 +6323,11 @@ pf_test(sa_family_t af, int fwdir, struc
> if (!pf_status.running)
> return (PF_PASS);
>
> +#if NCARP > 0
> if (ifp->if_type == IFT_CARP && ifp->if_carpdev)
> kif = (struct pfi_kif *)ifp->if_carpdev->if_pf_kif;
> else
> +#endif
> kif = (struct pfi_kif *)ifp->if_pf_kif;
>
> if (kif == NULL) {
>
