Francois Romieu wrote:
Minor nits below.
[...]
@@ -484,13 +484,32 @@
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) {
+ /* The skb memory should still be valid since we made a
copy,
+ * so can return error code here.
+ */
+ return rv;
+ }
+ else {
+ /* 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 0;
+ }
+ }
Why not use a single return point, say:
struct sk_buff *skb2 = skb_get(skb);
int rv = dev_queue_xmit(skb2);
if (!rv) {
/*
* 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;
That should be OK. I think I was worried that I would have to further
translate the error codes. There appears to be no documentation on what
valid return values are for the dev_queue_xmit or the hard_start_xmit
method... I think someone should add comments to the netdevice.h
file to specify the proper return codes (maybe even return an enum).
In my own code, I have this patch to dev.c. I think it is correct,
but I could be wrong:
@@ -1253,6 +1272,17 @@
* A negative errno code is returned on a failure. A success does not
* guarantee the frame will be transmitted as it may be dropped due
* to congestion or traffic shaping.
+ *
+ *
-----------------------------------------------------------------------------------
+ * I notice this method can also return errors from the queue disciplines,
+ * including NET_XMIT_DROP, which is a positive value. So, errors can
also
+ * be positive.
+ *
+ * Regardless of the return value, the skb is consumed, so it is currently
+ * impossible to retry a send to this method. This implies that virtual
devices,
+ * such as VLANs, can ONLY return 0 from their hard_start_xmit method,
making
+ * it difficult to apply pressure back up the stack.
+ * --Ben
*/
int dev_queue_xmit(struct sk_buff *skb)
vlan_dev_get_realdev_name() and vlan_dev_get_vid() can be tidy up
a bit (factoring dev_put() or handling debug code differently for instance).
Agreed, will fix this and the white-space issue.
Ben
--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc http://www.candelatech.com
|