Date: Fri, 18 Jul 2014 15:49:10 +0800 From: Marcelo Araujo <araujobsdport@gmail.com> To: Adrian Chadd <adrian@freebsd.org> Cc: FreeBSD Net <freebsd-net@freebsd.org> Subject: Re: [patch][lagg] - Set a better granularity and distribution on roundrobin protocol. Message-ID: <CAOfEmZhtZCettzD6pKQMHRiQE42nQmBuimOq28cA23R%2BYyc13w@mail.gmail.com> In-Reply-To: <CAOfEmZj5pk7bFB-PBqaJsi%2BbA73gbsUZzqggs4yEVky3_61NpQ@mail.gmail.com> References: <CAOfEmZjmb1bdvn0gR6vD1WeP8o8g7KwXod4TE0iJfa=nicyeng@mail.gmail.com> <CAJ-Vmomt2QDXAVBVUk6m8oH4Pa5yErDdG6wWrP3X7%2BDW137xiA@mail.gmail.com> <CAOfEmZja8Tkv_xG8LyR5Nbj%2BOga=vvdy=b3pxHqZi0-BBq25Uw@mail.gmail.com> <CAJ-VmomY2wP1EyVK4J16sGmMid=sJ9MPZrUY6pgcKGBDXm1T4g@mail.gmail.com> <CAOfEmZj5pk7bFB-PBqaJsi%2BbA73gbsUZzqggs4yEVky3_61NpQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] Hello guys, I made few changes on the lagg(4) patch. Also, I made tests using igb(4), ixgbe(4) and em(4); seems everything worked pretty well. I'm wondering if anyone else could make a review, and what I need to do, to see this patch committed. Best Regards, 2014-06-24 10:40 GMT+08:00 Marcelo Araujo <araujobsdport@gmail.com>: > > > 2014-06-24 6:54 GMT+08:00 Adrian Chadd <adrian@freebsd.org>: > > Hi, >> >> No, don't introduce out of order behaviour. Ever. > > > Yes, it has out of order behavior; with my patch much less. I upload two > pcap files and you can see by yourself, if you don't believe in what I'm > talking about. > > Test done using: "iperf -s" and "iperf -c <ip> -i 1 -t 10". > > 1) Don't change the number of packets(default round robin behavior). > http://people.freebsd.org/~araujo/lagg/lagg-nop.cap > 8 out of order packets. > Several SACKs. > > 2) Set the number of packets to 50. > http://people.freebsd.org/~araujo/lagg/lagg.cap > 0 out of order packets. > Less SACKs. > > >> You may not think >> it's a problem for TCP, but UDP things and VPN things will start >> getting very angry. There are VPN configurations out there that will >> drop the VPN if frames are out of order. >> > > I'm not thinking that will be a problem for TCP, but, in somehow it will > be, less throughput as I showed before, and less SACK. About the VPN, > please, tell me which softwares, and let me know where I can get a sample > to make a testbed. > > However to be very honest, I don't believe anyone here when change > something at network protocols will make this extensive testbed. It is > almost impossible to predict what software it will works or not, and I > don't believe anyone here has all these stuff in hands. > > >> >> The ixgbe driver is setting the flowid to the msix queue ID, rather >> than a 32 bit unique flow id hash value for the flow. That makes it >> hard to do traffic distribution where the flowid is available. >> > > Thanks for the explanation. > > >> >> There's an lagg option to re-hash the mbuf rather than rely on the >> flowid for outbound port choice - have you looked at using that? Did >> that make any difference? >> > > Yes, I set to 0 the net.link.lagg.0.use _flowid, it make a little > difference to the default round robin implementation, but yet I can't reach > more than 5 Gbit/s. With my patch and set the packets to 50, it improved a > bit too. > > So, thank you so much for all review, I don't know if you have time and a > testbed to make a real test, as I'm doing. I would be happy if you or more > people could make tests on that patch. Also, I have only ixgbe(4) to make > tests, would appreciate if this patch could be tested with other NICs too. > > Best Regards, > > -- > Marcelo Araujo (__) > araujo@FreeBSD.org \\\'',)http://www.FreeBSD.org <http://www.freebsd.org/>; \/ \ ^ > Power To Server. .\. /_) > > -- -- Marcelo Araujo (__)araujo@FreeBSD.org \\\'',)http://www.FreeBSD.org <http://www.freebsd.org/>; \/ \ ^ Power To Server. .\. /_) [-- Attachment #2 --] Index: if_lagg.c =================================================================== --- if_lagg.c (revision 268832) +++ if_lagg.c (working copy) @@ -187,6 +187,10 @@ SYSCTL_INT(_net_link_lagg, OID_AUTO, default_flowid_shift, CTLFLAG_RWTUN, &def_flowid_shift, 0, "Default setting for flowid shift for load sharing"); +static int lagg_rr_packets = 0; /* Default value for using rr_packets */ +SYSCTL_INT(_net_link_lagg, OID_AUTO, rr_packets, CTLFLAG_RW, + &lagg_rr_packets, 0, + "How many packets to be send per interface"); static int lagg_modevent(module_t mod, int type, void *data) @@ -1687,14 +1691,73 @@ { struct lagg_port *lp; uint32_t p; + uint32_t p2; + uint32_t pkt_sysctl_count; + int ifp_count = 1; p = atomic_fetchadd_32(&sc->sc_seq, 1); p %= sc->sc_count; + + p2 = atomic_fetchadd_32(&sc->sc_seq, 1); + p2 %= sc->sc_count; + lp = SLIST_FIRST(&sc->sc_ports); - while (p--) - lp = SLIST_NEXT(lp, lp_entries); /* + * If there is no reference for the IFP, we must + * copy it now. + */ + if (strlen(sc->sc_ref_ifp) == 0) + strncpy(sc->sc_ref_ifp, lp->lp_ifp->if_xname, sizeof(sc->sc_ref_ifp)); + + /* + * If ifp_count was not yet initialized, we must + * initialize now. + */ + if (sc->sc_ifp_count == 0) + sc->sc_ifp_count = 1; + + /* + * If the sysctl rr_packets is set to 0, we must use the + * roundrobin as it is, or otherwise, we must apply the + * granularity between the interfaces that are part of the group. + */ + if (!lagg_rr_packets) { + while (p--) + lp = SLIST_NEXT(lp, lp_entries); + goto send_mbuf; + } else { + pkt_sysctl_count = atomic_fetchadd_32(&sc->sc_pkt_count, 1); + if (pkt_sysctl_count == lagg_rr_packets) { + if (sc->sc_ifp_count <= sc->sc_count) { + while (ifp_count < sc->sc_ifp_count) { + lp = SLIST_NEXT(lp, lp_entries); + ifp_count++; + } + sc->sc_ifp_count++; + if (sc->sc_ifp_count > sc->sc_count) + sc->sc_ifp_count = 0; + } + strncpy(sc->sc_ref_ifp, lp->lp_ifp->if_xname, sizeof(sc->sc_ref_ifp)); + sc->sc_pkt_count = 0; + } + } + + /* + * Check if the current interface to be enqueue is not the + * same used in the last round. + */ + lp = SLIST_FIRST(&sc->sc_ports); + while (p2--) { + if (strcmp(lp->lp_ifp->if_xname, sc->sc_ref_ifp) == 0) + break; + else + lp = SLIST_NEXT(lp, lp_entries); + } + goto send_mbuf; + +send_mbuf: + /* * Check the port's link state. This will return the next active * port if the link is down or the port is NULL. */ Index: if_lagg.h =================================================================== --- if_lagg.h (revision 268832) +++ if_lagg.h (working copy) @@ -232,6 +232,9 @@ struct sysctl_oid *sc_oid; /* sysctl tree oid */ int use_flowid; /* use M_FLOWID */ int flowid_shift; /* shift the flowid */ + uint32_t sc_pkt_count; /* use for count packates per ifp */ + int sc_ifp_count; /* counter reference of interfaces on rr */ + char sc_ref_ifp[IFNAMSIZ]; /* name of the ifp */ }; struct lagg_port {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAOfEmZhtZCettzD6pKQMHRiQE42nQmBuimOq28cA23R%2BYyc13w>