On Wed, Jan 15, 2014 at 05:17:02PM +0900, Simon Horman wrote:
> If VLAN acceleration is used when the kernel receives a packet
> then the outer-most VLAN tag will not be present in the packet
> when it is received by netdev-linux. Rather, it will be present
> in auxdata.
>
> This patch uses recvmsg() instead of recv() to read auxdata for
> each packet and if the vlan_tid is set then it is added to the packet.
>
> Adding the vlan_tid makes use of headroom available
> in the buffer parameter of rx_recv.
>
> Co-authored-by: Ben Pfaff <b...@nicira.com>
> Signed-off-by: Simon Horman <ho...@verge.net.au>
I noticed one minor bug (ETH_ADDR_LEN should be ETH_HEADER_LEN) and a
number of opportunities for minor improvements. I'm folding in the
following, and then I'll apply it to master.
Thank you!
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 4bd4bfc..9c1a36d 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -809,9 +809,7 @@ netdev_linux_rx_construct(struct netdev_rx *rx_)
}
val = 1;
- error = setsockopt(rx->fd, SOL_PACKET, PACKET_AUXDATA,
- &val, sizeof val);
- if (error) {
+ if (setsockopt(rx->fd, SOL_PACKET, PACKET_AUXDATA, &val, sizeof val)) {
error = errno;
VLOG_ERR("%s: failed to mark socket for auxdata (%s)",
netdev_get_name(netdev_), ovs_strerror(error));
@@ -946,9 +944,9 @@ netdev_linux_rx_recv_sock(int fd, struct ofpbuf *buffer)
continue;
}
- aux = (struct tpacket_auxdata *)(void *)CMSG_DATA(cmsg);
+ aux = ALIGNED_CAST(struct tpacket_auxdata *, CMSG_DATA(cmsg));
if (auxdata_has_vlan_tci(aux)) {
- if (retval < ETH_ADDR_LEN) {
+ if (retval < ETH_HEADER_LEN) {
return EINVAL;
}
@@ -990,11 +988,9 @@ netdev_linux_rx_recv(struct netdev_rx *rx_, struct ofpbuf
*buffer)
retval = (rx->is_tap
? netdev_linux_rx_recv_tap(rx->fd, buffer)
: netdev_linux_rx_recv_sock(rx->fd, buffer));
- if (retval) {
- if (retval != EAGAIN && retval != EMSGSIZE) {
- VLOG_WARN_RL(&rl, "error receiving Ethernet packet on %s: %s",
- ovs_strerror(errno), netdev_rx_get_name(rx_));
- }
+ if (retval && retval != EAGAIN && retval != EMSGSIZE) {
+ VLOG_WARN_RL(&rl, "error receiving Ethernet packet on %s: %s",
+ ovs_strerror(errno), netdev_rx_get_name(rx_));
}
return retval;
diff --git a/lib/netdev.c b/lib/netdev.c
index 66b411a..8e62421 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -500,6 +500,13 @@ netdev_parse_name(const char *netdev_name_, char **name,
char **type)
}
}
+/* Attempts to open a netdev_rx handle for obtaining packets received on
+ * 'netdev'. On success, returns 0 and stores a nonnull 'netdev_rx *' into
+ * '*rxp'. On failure, returns a positive errno value and stores NULL into
+ * '*rxp'.
+ *
+ * Some kinds of network devices might not support receiving packets. This
+ * function returns EOPNOTSUPP in that case.*/
int
netdev_rx_open(struct netdev *netdev, struct netdev_rx **rxp)
OVS_EXCLUDED(netdev_mutex)
@@ -531,6 +538,7 @@ netdev_rx_open(struct netdev *netdev, struct netdev_rx
**rxp)
return error;
}
+/* Closes 'rx'. */
void
netdev_rx_close(struct netdev_rx *rx)
OVS_EXCLUDED(netdev_mutex)
@@ -543,6 +551,29 @@ 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.
+ *
+ * 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.
+ *
+ * It is advised that the tailroom of 'buffer' should be
+ * VLAN_HEADER_LEN bytes longer than the MTU to allow space for an
+ * out-of-band VLAN header to be added to the packet. At the very least,
+ * 'buffer' must have at least ETH_TOTAL_MIN bytes of tailroom.
+ *
+ * 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)
{
@@ -563,12 +594,15 @@ netdev_rx_recv(struct netdev_rx *rx, struct ofpbuf
*buffer)
}
}
+/* Arranges for poll_block() to wake up when a packet is ready to be received
+ * on 'rx'. */
void
netdev_rx_wait(struct netdev_rx *rx)
{
rx->netdev->netdev_class->rx_wait(rx);
}
+/* Discards any packets ready to be received on 'rx'. */
int
netdev_rx_drain(struct netdev_rx *rx)
{
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
