On 22/06/2011, at 3:52 AM, Stefan Rinkes wrote:
> Hi,
>
> while playing around with carp and pfsync I spotted
> two minor bugs.
>
> 1. Not all pfstate flags are synced, cause pfsync uses
> u_int8_t, while pf uses u_int16_t for state_flags.
> Currently that means PFSTATE_SCRUB_TCP flags don't
> get synced.
>
> retrieving revision 1.333
> diff -u -p -r1.333 pfvar.h
> --- sys/net/pfvar.h 20 Jun 2011 19:03:41 -0000 1.333
> +++ sys/net/pfvar.h 21 Jun 2011 17:33:31 -0000
> @@ -892,13 +892,13 @@ struct pfsync_state {
> u_int8_t proto;
> u_int8_t direction;
> u_int8_t log;
> - u_int8_t state_flags;
> + u_int16_t state_flags;
> u_int8_t timeout;
> u_int8_t sync_flags;
> u_int8_t updates;
> u_int8_t min_ttl;
> u_int8_t set_tos;
> - u_int8_t pad[4];
> + u_int8_t pad[3];
> } __packed;
>
> #define PFSYNC_FLAG_SRCNODE 0x04
this diff is not ok.
you changed the wire format, didnt considering endianness, and it looks like
you put a multibyte value on an unaligned boundary.
i'll have a look at the missing flags problem myself in the next few days.
> 2. If you are using IP balanced carp and set one of
> the interface to down, the mbufs still reach pf.
> Cause carp_ourether() returns NULL if the interface
> is down and the mbufs gets copied by carp_input(),
> cause the M_MCAST flag is set. The copied mbuf is
> dropped in ether_input() since the carp interface is down
> and the original mbuf reaches the pf. IMHO carp should always
> take care of mbufs with his MAC address, else the machine has
> to do some unnecessary work.
>
> retrieving revision 1.184
> diff -u -p -r1.184 ip_carp.c
> --- sys/netinet/ip_carp.c 4 May 2011 16:05:49 -0000 1.184
> +++ sys/netinet/ip_carp.c 21 Jun 2011 17:34:42 -0000
> @@ -1514,9 +1514,7 @@ carp_ourether(void *v, struct ether_head
>
> TAILQ_FOREACH(vh, &cif->vhif_vrs, sc_list) {
> struct carp_vhost_entry *vhe;
> - if ((vh->sc_if.if_flags & (IFF_UP|IFF_RUNNING)) !=
> - (IFF_UP|IFF_RUNNING))
> - continue;
> +
> if (vh->sc_balancing == CARP_BAL_ARP) {
> LIST_FOREACH(vhe, &vh->carp_vhosts, vhost_entries)
> if (vhe->state == MASTER &&
>
this looks reasonable to me. mcbride, mpf, could you chip in on this?
dlg
