On Fri, May 29, 2009 at 8:22 PM, Anton Maksimenkov <[email protected]> wrote: > I need some help with my idea about PF. I want to implement idea based > on freebsd's ipfw "mask dst-ip" feature. > > Lets assume that we have some altq rules like this: > altq on $if cbq bandwidth 100Mb queue { std, ..., pq } > queue ... > queue pq bandwidth ... priority 1 cbq { pq_1, pq_2, pq_3,...} > queue pq_1 bandwidth 256K > queue pq_2 bandwidth 256K > queue pq_3 bandwidth 256K > ... > And some filter rule like this: > pass out on $if ... from any to 192.168.0.0/24 ... queue pq > > So, assume that we want to assign all packets passed that rule to one > of _subqueues_ of "pq" queue. Not to "pq" itself. That assignment must > be based on packet's destination IP address, so packets to 192.168.0.1 > must be assigned to "pq_1" queue, packets to 192.168.0.2 - to "pq_2", > and so on. > > I "implement" that idea (my diff is totally wrong, but it only have to > show my idea). I started with imagination that instead of assigning > rule's queue tag to packet I try to find some subqueue of rule's > queue. > (On example above, instead of assigning "pq" queue for packet to > 192.168.0.1 I seek for "pq_1" queue.) > > Again, my "implementation" is very ugly, it's becouse I'm not very > familiar with PF code yet. Here is the diff: > Index: pf.c > =================================================================== > RCS file: /cvs/src/sys/net/pf.c,v > retrieving revision 1.634.2.1 > diff -u pf.c > --- pf.c 11 Apr 2009 23:43:40 -0000 1.634.2.1 > +++ pf.c 29 May 2009 17:55:29 -0000 > @@ -5287,8 +5287,30 @@ > if (action == PF_PASS && r->qid) { > if (pqid || (pd.tos & IPTOS_LOWDELAY)) > m->m_pkthdr.pf.qid = r->pqid; > - else > - m->m_pkthdr.pf.qid = r->qid; > + else { > + /* m->m_pkthdr.pf.qid = r->qid; */ > + char qname[PF_TAG_NAME_SIZE], > qsubname[PF_TAG_NAME_SIZE]; > + u_int32_t i_pkt_dstip, i_rule_dstnet, i_diff; > + struct pf_tagname *tag; > + /* packet's dst IP ==> u_int32_t */ > + i_pkt_dstip = ntohl(pd.dst->pfa.v4.s_addr); > + /* rule's dst (net) IP ==> u_int32_t */ > + i_rule_dstnet = ntohl(r->dst.addr.va.addr.s_addr); > + i_diff = i_pkt_dstip - i_rule_dstnet; > + /* get rule's queue name by queue tag id */ > + tag2tagname(&pf_tags, r->qid, qname); > + /* create sub-queue name on packet's dst IP basis */ > + if (snprintf(qsubname, sizeof(qsubname), > "%s_%u", qname, i_diff) == -1) > + panic("Can't create subqueue name"); > + /* get tag of that queue */ > + TAILQ_FOREACH(tag, &pf_tags, entries) > + if (strcmp(qsubname, tag->name) == 0) { > + /* assign packet to this sub-queue */ > + m->m_pkthdr.pf.qid = tag->tag; > + break; > + } > + > + } > /* add hints for ecn */ > m->m_pkthdr.pf.hdr = h; > } > > Yes, yes, it's totally wrong implemented, it uses structure members > wrong, it uses "not existing here" variables etc, of cause. Pls, don't > kick me much, I just want to discuss the idea at first. > > So, at first, is the idea in general acceptable?
You can try to finish this patch here: http://redmine.pfsense.org/repositories/entry/pfsense-tools/patches/misc/altq _wfq.diff It gives WFQ properties to HFSC/PRIQ cause CBQ has it already. It is supposed to work like the red/ecn/rio option. The only things missing is the hashing on pf side and the userland code for this. So than you can configure the queues to really behave like a mask by defining the hashing you need by src-ip/dst-ip/etc.... -- Ermal
