jamal wrote:
On Wed, 2005-28-12 at 18:19 +0100, Patrick McHardy wrote:
You seem to add almost as much ifdefed code to
dummy as it currently has, which makes me wonder if a completely
new device wouldn't be much cleaner.
Its one of those 50/50 decisions - I could create a new device,
what would you think for a name?
IMQ? :) No, I'm bad coming up with names myself ..
Note that: The existence of dummy is for very simple reasons - to
blackhole packets. It has all the infrastructure code needed; I just
added one optional use. Dummy hasnt been touched in > 10 years, someone
needed to do it ;->
Doesn't the blackholing conflict with this new use?
+ __u32 from = G_TC_FROM(skb->tc_verd);
+ if (!from || !skb->input_dev ) {
In dev_queue_xmit both from and input_dev should be set even
for normal blackholed packets, unless I'm missing something.
jamal wrote:
+ /* mostly debug stats leave in for now */
+ unsigned long stat_te; /* tasklet entered */
+ unsigned long stat_tqr; /* transmit queue refill attempt */
+ unsigned long stat_rqe; /* receive queue entered */
[..]
I would prefer more descriptive names, like tasklet_entered,
txqueue_refill_attempt, ...
I had the names as long as you describe them and eventually abbreviated
them because it was getting annoying to space them. Arent the comments
sufficient? note i eventually would like to kill these stats in any
case. I have them in there as performance measurement tool.
Yes, but that means I need to search for the comments if I want to
know what "rqe" is. You can eliminate the need for the comments
by using more descriptive names.
+ } else {
+ /* if netfilt is compiled in and packet is
+ tagged, we could reinject the packet back
+ this would make it do remaining 10%
+ of what current IMQ does
+ if someone really really insists then
+ this is the spot .. jhs */
+ dev_kfree_skb(skb);
+ stats->tx_dropped++;
Packets that are queued from netfilter must not be freed other
than by passing them to nf_reinject with a verdict of drop.
It doesn't look as if you were using netfilter at all, but
at least that comment is wrong.
That just points to a spot where the change could be made,
not necessarily how it is done;-> If ever netfilter was added, it would
have to be done properly. Does this sound reasonable?
I could add more comments to the effect which you specified.
I'd say just kill the comment, I don't see any advantage in using
netfilter for this.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
