[ovs-dev] [PATCH 2/9] netdev: Send ofpuf directly to netdev.
Pravin Shelar
pshelar at nicira.com
Wed Mar 19 03:38:37 UTC 2014
On Tue, Mar 18, 2014 at 4:04 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>
> On Mar 18, 2014, at 1:53 PM, Pravin <pshelar at nicira.com> wrote:
>
>> DPDK netdev need to access ofpbuf while sending buffer. Following
>> patch changes netdev_send accordingly.
>
> Would it still be possible to not transfer the responsibility of freeing the buffer? This relates to the comments I sent for patch 1.
>
I need to do that for better performance.
>>
>> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
>> ---
>> lib/dpif-netdev.c | 9 +++++----
>> lib/netdev-dummy.c | 7 ++++++-
>> lib/netdev-linux.c | 9 ++++++++-
>> lib/netdev-provider.h | 14 +++++++-------
>> lib/netdev.c | 4 ++--
>> lib/netdev.h | 2 +-
>> 6 files changed, 29 insertions(+), 16 deletions(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index d78fb25..440561e 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -1806,7 +1806,7 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet,
>> case OVS_ACTION_ATTR_OUTPUT:
>> p = dp_netdev_lookup_port(aux->dp, u32_to_odp(nl_attr_get_u32(a)));
>> if (p) {
>> - netdev_send(p->netdev, packet);
>> + netdev_send(p->netdev, packet, may_steal);
>> }
>> break;
>>
>> @@ -1817,6 +1817,10 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet,
>>
>> dp_netdev_output_userspace(aux->dp, packet, DPIF_UC_ACTION, aux->key,
>> userdata);
>> +
>> + if (may_steal) {
>> + ofpbuf_delete(packet);
>> + }
>> break;
>> }
>> case OVS_ACTION_ATTR_PUSH_VLAN:
>> @@ -1830,9 +1834,6 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet,
>> OVS_NOT_REACHED();
>> }
>>
>> - if (may_steal) {
>> - ofpbuf_delete(packet);
>> - }
>> }
>>
>> static void
>> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
>> index 27487f9..7c73ca6 100644
>> --- a/lib/netdev-dummy.c
>> +++ b/lib/netdev-dummy.c
>> @@ -817,9 +817,11 @@ netdev_dummy_rx_drain(struct netdev_rx *rx_)
>> }
>>
>> static int
>> -netdev_dummy_send(struct netdev *netdev, const void *buffer, size_t size)
>> +netdev_dummy_send(struct netdev *netdev, struct ofpbuf *pkt, bool may_steal)
>> {
>> struct netdev_dummy *dev = netdev_dummy_cast(netdev);
>> + const void *buffer = pkt->data;
>> + size_t size = pkt->size;
>>
>> if (size < ETH_HEADER_LEN) {
>> return EMSGSIZE;
>> @@ -854,6 +856,9 @@ netdev_dummy_send(struct netdev *netdev, const void *buffer, size_t size)
>> }
>>
>> ovs_mutex_unlock(&dev->mutex);
>> + if (may_steal) {
>> + ofpbuf_delete(pkt);
>> + }
>>
>> return 0;
>> }
>> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
>> index 574a572..a6e2217 100644
>> --- a/lib/netdev-linux.c
>> +++ b/lib/netdev-linux.c
>> @@ -1049,8 +1049,11 @@ netdev_linux_rx_drain(struct netdev_rx *rx_)
>> * The kernel maintains a packet transmission queue, so the caller is not
>> * expected to do additional queuing of packets. */
>> static int
>> -netdev_linux_send(struct netdev *netdev_, const void *data, size_t size)
>> +netdev_linux_send(struct netdev *netdev_, struct ofpbuf *pkt, bool may_steal)
>> {
>> + const void *data = pkt->data;
>> + size_t size = pkt->size;
>> +
>> for (;;) {
>> ssize_t retval;
>>
>> @@ -1122,6 +1125,10 @@ netdev_linux_send(struct netdev *netdev_, const void *data, size_t size)
>> return 0;
>> }
>> }
>> +
>> + if (may_steal) {
>> + ofpbuf_delete(pkt);
>> + }
>> }
>>
>> /* Registers with the poll loop to wake up from the next call to poll_block()
>> diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
>> index 9bacaa0..90700db 100644
>> --- a/lib/netdev-provider.h
>> +++ b/lib/netdev-provider.h
>> @@ -223,13 +223,13 @@ struct netdev_class {
>> const struct netdev_tunnel_config *
>> (*get_tunnel_config)(const struct netdev *netdev);
>>
>> - /* Sends the 'size'-byte packet in 'buffer' on 'netdev'. Returns 0 if
>> - * successful, otherwise a positive errno value. Returns EAGAIN without
>> - * blocking if the packet cannot be queued immediately. Returns EMSGSIZE
>> - * if a partial packet was transmitted or if the packet is too big or too
>> - * small to transmit on the device.
>> + /* Sends the buffer->data of size buffer->sizefrom 'buffer' on 'netdev'.
>
> Missing space.
>
ok.
>> + * Returns 0 if successful, otherwise a positive errno value. Returns
>> + * EAGAIN without blocking if the packet cannot be queued immediately.
>> + * Returns EMSGSIZE if a partial packet was transmitted or if the packet
>> + * is too big or too small to transmit on the device.
>> *
>> - * The caller retains ownership of 'buffer' in all cases.
>> + * The caller retains ownership of 'buffer' if may_steal is true.
>
> This looks like a reverse of your usage.
>
right, I went back and forth on this. I will update comment.
>> *
>> * The network device is expected to maintain a packet transmission queue,
>> * so that the caller does not ordinarily have to do additional queuing of
>> @@ -241,7 +241,7 @@ struct netdev_class {
>> * network device from being usefully used by the netdev-based "userspace
>> * datapath". It will also prevent the OVS implementation of bonding from
>> * working properly over 'netdev'.) */
>> - int (*send)(struct netdev *netdev, const void *buffer, size_t size);
>> + int (*send)(struct netdev *netdev, struct ofpbuf *buffer, bool may_steal);
>>
>> /* Registers with the poll loop to wake up from the next call to
>> * poll_block() when the packet transmission queue for 'netdev' has
>> diff --git a/lib/netdev.c b/lib/netdev.c
>> index a058742..295e121 100644
>> --- a/lib/netdev.c
>> +++ b/lib/netdev.c
>> @@ -610,12 +610,12 @@ netdev_rx_drain(struct netdev_rx *rx)
>> * Some network devices may not implement support for this function. In such
>> * cases this function will always return EOPNOTSUPP. */
>> int
>> -netdev_send(struct netdev *netdev, const struct ofpbuf *buffer)
>> +netdev_send(struct netdev *netdev, struct ofpbuf *buffer, bool may_steal)
>> {
>> int error;
>>
>> error = (netdev->netdev_class->send
>> - ? netdev->netdev_class->send(netdev, buffer->data, buffer->size)
>> + ? netdev->netdev_class->send(netdev, buffer, may_steal)
>> : EOPNOTSUPP);
>> if (!error) {
>> COVERAGE_INC(netdev_sent);
>> diff --git a/lib/netdev.h b/lib/netdev.h
>> index bfd8f91..2b4e2a9 100644
>> --- a/lib/netdev.h
>> +++ b/lib/netdev.h
>> @@ -166,7 +166,7 @@ void netdev_rx_wait(struct netdev_rx *);
>> int netdev_rx_drain(struct netdev_rx *);
>>
>> /* Packet transmission. */
>> -int netdev_send(struct netdev *, const struct ofpbuf *);
>> +int netdev_send(struct netdev *, struct ofpbuf *, bool may_steal);
>> void netdev_send_wait(struct netdev *);
>>
>> /* Hardware address. */
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
More information about the dev
mailing list