On 31/08/13(Sat) 04:28, Nathanael Rensen wrote:
> If no flowsrc is specified on a pflow(4) interface then the src address
> is determined by ip_output(). However prior to calling ip_output() pflow(4)
> has already calculated the UPD pseudo-header checksum based on INADDR_ANY.
> This results in a bad UPD checksum and the resulting pflow packet is
> rejected by the receiver.
>
> The diff below resolves this by calling in_selectsrc() if flowsrc is not
> specified.
I'm not sure we want to add yet-another different chunk of code to
determine a source address, especially when I'm working on reducing
their number ;)
I'm not familiar with pflow(4), but is there any advantage of not
specifying a flowsrc target? Because reducing the number of code
path passing an INADDR_ANY source address would simplify a lot of
our address lookups.
Otherwise did you considered deferring the checksum?
> Index: if_pflow.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_pflow.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 if_pflow.c
> --- if_pflow.c 13 Aug 2013 08:44:05 -0000 1.34
> +++ if_pflow.c 30 Aug 2013 18:54:03 -0000
> @@ -1512,8 +1512,37 @@ pflow_sendout_mbuf(struct pflow_softc *s
> struct ifnet *ifp = &sc->sc_if;
> #endif
> struct ip *ip;
> + struct in_addr sender_ip;
> int err;
>
> + /* Determine the sender address */
> + sender_ip = sc->sc_sender_ip;
> + if (sender_ip.s_addr == INADDR_ANY) {
> + struct sockaddr_in sin;
> + struct sockaddr_in *ifaddr;
> + struct route ro;
> +
> + bzero(&ro, sizeof(ro));
> + bzero(&sin, sizeof(sin));
> + sin.sin_len = sizeof(sin);
> + sin.sin_family = AF_INET;
> + sin.sin_port = sc->sc_receiver_port;
> + sin.sin_addr = sc->sc_receiver_ip;
> +
> + err = 0;
> + ifaddr = in_selectsrc(&sin, &ro, 0, NULL, &err, 0);
> + if (ifaddr == NULL) {
> + if (err == 0)
> + err = EADDRNOTAVAIL;
> + return err;
> + }
> +
> + sender_ip = ifaddr->sin_addr;
> +
> + if (ro.ro_rt)
> + rtfree(ro.ro_rt);
> + }
> +
> /* UDP Header*/
> M_PREPEND(m, sizeof(struct udpiphdr), M_DONTWAIT);
> if (m == NULL) {
> @@ -1523,7 +1552,7 @@ pflow_sendout_mbuf(struct pflow_softc *s
>
> ui = mtod(m, struct udpiphdr *);
> ui->ui_pr = IPPROTO_UDP;
> - ui->ui_src = sc->sc_sender_ip;
> + ui->ui_src = sender_ip;
> ui->ui_sport = sc->sc_sender_port;
> ui->ui_dst = sc->sc_receiver_ip;
> ui->ui_dport = sc->sc_receiver_port;
>
