Yeah, this sounds like a reasonable implementation to me. Cliff
On Wed, Feb 16, 2011 at 1:00 PM, Philip Prindeville < [email protected]> wrote: > Why not just keep a linked list of all the ARPQuerier instances that an > ARPTable is associated with it, and have it iterate over the list? > > In the nominal case, that's going to be a single instance of ARPQuerier > anyway, so there's no overhead there... > > And in my case, I was having to pump a packet either through Tee(n) or else > RandomSwitch(n) to and deliver it to all or one load-balanced instances... > which I'm guessing is of at least comparable overhead. > > > > On 2/16/11 12:46 PM, Cliff Frey wrote: > >> I would say that this is a bug by some definition, but it is a very tricky >> one to solve. Specifically, even if you did change the queueing to be in >> arpquerier, you would still need a pointer back from arptable -> arpquerier >> so that if an arp response came in on one querier that updated the arptable, >> the arptable would have to wake up the other arpquerier which had a queued >> packet. >> >> I think that most people can get around it by just using different >> arptables for each arpquerier. >> >> Cliff >> >> On Tue, Feb 15, 2011 at 1:33 PM, Philip Prindeville < >> [email protected] <mailto: >> [email protected]>> wrote: >> >> Thought about this some more, and it's going to be more of a challenge >> than I thought. >> >> Part of the problem is that the packets get queued up in the ARPTable, >> and not in the ARPQuerier, so if you have multiple ARPQuerier instances >> using the same ARPTable (we do... we're in a threaded/polled environment) >> then the packet loses the association to the ARPQuerier (and the >> ARPQuerier's output ports) when it gets queued. >> >> For that matter, I'm not sure that ARPQuerier::handle_response() is >> correct: all of the packets get put onto the same queue in the ARPTable, so >> that when the queue gets drained they might not go out on the output ports >> of the same instance of ARPQuerier that the came in on. >> >> Example: >> >> o1 :: ... >> o2 :: ... >> at :: ARPTable(); >> >> ... -> a1 :: ARPQuerier(TABLE at) -> o1; >> ... -> a2 :: ARPQuerier(TABLE at) -> o2; >> >> now let's say that packet p1 comes in on a1, and packet p2 comes in on >> a2. Both are destined to x.x.x.x. >> >> Then an ARP response comes back resolving x.x.x.x. >> >> Both packets will be sent to o1, or o2, depending on whether the answer >> is delivered to a1 or a2... >> >> So packets might be "switched" from the a1...o1 path to o2, or vice >> versa. >> >> Is this a bug? >> >> Should the queuing (and queue expiration) be happening in the >> ARPQuerier class instead? >> >> -Philip >> >> >> On 2/12/11 11:22 AM, Philip Prindeville wrote: >> > I wanted to make a change to ARPTable which would give it the ability >> to invoke a hook back to ARPQuerier when it deletes queued up packages, but >> wasn't sure if there's a clean way to do this. >> > >> > In C, I'd just set a pointer to a callback function, but this isn't >> C. >> > >> > So what's the best way to do it? Keep in mind that the ARPTable >> might have been >> > >> > Also, it's the case that the same ARPTable instance might be shared >> amongst several ARPQuerier instances (because we're in a multi-core >> environment with several threads running on the same NIC). >> > >> > Oh, one other question about ARPTable vs. ARPQuerier interactions... >> if I have that same situation where I'm running multithreaded, and a >> response packet gets delivered to one ARPQuerier, thereby updating the >> ARPTable... will the queued up packets in each instance of the ARPQueriers >> get updated and unblock the packets? >> > >> > Or are the queued packets actually held by the ARPTable (and >> therefore in a single place)? >> > >> > Looking at how ARPQuerier::handle_ip() and >> ARPQuerier::handle_response() interact, it looks like the packets are all >> held by the ARPTable, and when the queue gets drained, all drained by the >> same instance... so any affinity of individual packets to threads is lost. >> > >> > Packet *cached_packet; >> > arpt->insert(ipa, ena,&cached_packet); >> > >> > while (cached_packet) { >> > Packet *next = cached_packet->next(); >> > handle_ip(cached_packet, true); >> > cached_packet = next; >> > } >> > >> > >> > Correct? >> > >> > Thanks. >> > >> > -Philip >> >> _______________________________________________ >> click mailing list >> [email protected] <mailto:[email protected]> >> >> https://amsterdam.lcs.mit.edu/mailman/listinfo/click >> >> >> > _______________________________________________ click mailing list [email protected] https://amsterdam.lcs.mit.edu/mailman/listinfo/click
