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

Reply via email to