Ben Greear wrote:
--- linux-2.6.9/net/8021q/vlan_dev.c 2004-10-18 14:55:07.000000000 -0700
+++ linux-2.6.9.p4s/net/8021q/vlan_dev.c 2004-10-25 13:38:32.779294920 -0700
@@ -1,4 +1,4 @@
-/*
+/* -*- linux-c -*-
* INET 802.1Q VLAN
* Ethernet-type device handling.
*
@@ -484,13 +484,26 @@
veth->h_vlan_proto, veth->h_vlan_TCI, veth->h_vlan_encapsulated_proto);
#endif
- stats->tx_packets++; /* for statics only */
- stats->tx_bytes += skb->len;
-
skb->dev = VLAN_DEV_INFO(dev)->real_dev;
- dev_queue_xmit(skb);
- return 0;
+ {
+ /* Please note, dev_queue_xmit consumes the pkt regardless of the
+ * error value. So, will copy the skb first and free if successful.
+ */
+ struct sk_buff* skb2 = skb_get(skb);
+ int rv = dev_queue_xmit(skb2);
+ if (rv == 0) {
+ /* Was success, need to free the skb reference since we bumped up the
+ * user count above.
+ */
+
+ stats->tx_packets++; /* for statics only */
+ stats->tx_bytes += skb->len;
+
+ kfree_skb(skb);
+ }
+ return rv;
+ }
}
int vlan_dev_hwaccel_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
Hi Ben,
I am not happy with this change to vlan_dev_hard_start_xmit().
I certainly appreciate the idea of avoiding flow-control "black-holes", but
that would
take more than just this change. You have to consider (at least) these issues:
o It is considered an error if a queue-less device returns anything but zero
from its
hard_start_xmit() function (see dev_queue_xmit()).
o So, lets add a tx queue to it. Sure, that would be nice. Now we can even do
shaping
and other fancy stuff. But then how do we manage netif_queue_stopped?
Especially
restarting the queue could be tricky.
o But couldn't we skip netif_stop_queue() and just return NETDEV_TX_BUSY when
congested?
No, that would make the qdisc system "busy-retry" untill it succeeds. BAD.
o It is unsafe to pass a shared skb to dev_queue_xmit() unless you control all
the
references yourself. (It will likely be enqueued on a list.)
And specifically for this patch:
o The skb could be freed (replaced) in __vlan_put_tag(), so you cannot tell
the caller
to hang on to it.
o If rv is NET_XMIT_CN (and probably also rv < 0) you have to return 0, in
order to
make the caller forget about this skb.
Dave, I think this part of the patch should be reverted until someone comes
up with a
general scheme for flow_control handling through virtual devices.
-Tommy
|