On 03/21/2014 07:02 PM, Pravin wrote:
diff --git a/lib/automake.mk b/lib/automake.mk
@@ -1863,7 +1850,7 @@ dp_netdev_port_input(struct dp_netdev *dp, struct ofpbuf
*packet,
dp_netdev_flow_used(netdev_flow, packet, &key);
actions = dp_netdev_flow_get_actions(netdev_flow);
- dp_netdev_execute_actions(dp, &key, packet, md,
+ dp_netdev_execute_actions(dp, &key, packet, true, md,
actions->actions, actions->size);
I am not 100% sure I understand who is responsible for deleting the
packet, e.g. if the actions are not terminated with an output or
userspace. Or is that never possible?
dp_netdev_count_packet(dp, DP_STAT_HIT);
} else if (dp->handler_queues) {
@@ -1871,6 +1858,7 @@ dp_netdev_port_input(struct dp_netdev *dp, struct ofpbuf
*packet,
dp_netdev_output_userspace(dp, packet,
flow_hash_5tuple(&key, 0) % dp->n_handlers,
DPIF_UC_MISS, &key, NULL);
+ ofpbuf_delete(packet);
Given we now have to free packets if they are not stolen, the
if (packet->size < ETH_HEADER_LEN) check in dp_netdev_port_input()
seems to leak now. How about dp_netdev_port_input() returns bool
indicating whether packet was stolen or not so we can free it in a
central place?
diff --git a/lib/netdev.c b/lib/netdev.c
index e9c8d8f..0d7b69d 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -551,22 +551,13 @@ netdev_rx_close(struct netdev_rx *rx)
}
}
-/* Attempts to receive a packet from 'rx' into the tailroom of 'buffer', which
- * must initially be empty. If successful, returns 0 and increments
- * 'buffer->size' by the number of bytes in the received packet, otherwise a
- * positive errno value.
+/* Attempts to receive batch of packets from 'rx'.
*
* Returns EAGAIN immediately if no packet is ready to be received.
*
* Returns EMSGSIZE, and discards the packet, if the received packet is longer
* than 'ofpbuf_tailroom(buffer)'.
*
- * Implementations may make use of VLAN_HEADER_LEN bytes of tailroom to
- * add a VLAN header which is obtained out-of-band to the packet. If
- * this occurs then VLAN_HEADER_LEN bytes of tailroom will no longer be
- * available for the packet, otherwise it may be used for the packet
- * itself.
Any reason for removing this comment other than that it seems oudated?
The headroom containing space for an additional VLAN header should be
documented somewhere.
@@ -575,23 +566,15 @@ netdev_rx_close(struct netdev_rx *rx)
* This function may be set to null if it would always return EOPNOTSUPP
* anyhow. */
int
-netdev_rx_recv(struct netdev_rx *rx, struct ofpbuf *buffer)
+netdev_rx_recv(struct netdev_rx *rx, struct ofpbuf **buffers, int *cnt)
{
int retval;
- ovs_assert(buffer->size == 0);
- ovs_assert(ofpbuf_tailroom(buffer) >= ETH_TOTAL_MIN);
-
- retval = rx->netdev->netdev_class->rx_recv(rx, buffer);
+ retval = rx->netdev->netdev_class->rx_recv(rx, buffers, cnt);
if (!retval) {
COVERAGE_INC(netdev_received);
What would be awesome here is a COVERAGE_ADD(netdev_batched, cnt).
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
