On Tue, Mar 18, 2014 at 4:04 PM, Jarno Rajahalme <[email protected]> wrote: > > On Mar 18, 2014, at 1:53 PM, Pravin <[email protected]> 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 <[email protected]> >> --- >> 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 >> [email protected] >> http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
