Dmitry Eremin-Solenikov(lumag) replied on github web page:
platform/linux-generic/pktio/null.c
line 135
@@ -0,0 +1,214 @@
+/* Copyright (c) 2017, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#include "config.h"
+
+#include <odp_posix_extensions.h>
+
+#include <sys/socket.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <linux/if_packet.h>
+#include <linux/filter.h>
+#include <ctype.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <bits/wordsize.h>
+#include <net/ethernet.h>
+#include <netinet/ip.h>
+#include <arpa/inet.h>
+#include <stdint.h>
+#include <string.h>
+#include <net/if.h>
+#include <inttypes.h>
+#include <poll.h>
+#include <sys/ioctl.h>
+#include <errno.h>
+#include <sys/syscall.h>
+#include <linux/ethtool.h>
+#include <linux/sockios.h>
+
+#include <odp_api.h>
+#include <odp_packet_socket.h>
+#include <odp_packet_internal.h>
+#include <odp_packet_io_internal.h>
+#include <odp_align_internal.h>
+#include <odp_debug_internal.h>
+#include <odp_classification_datamodel.h>
+#include <odp_classification_inlines.h>
+#include <odp_classification_internal.h>
+#include <odp/api/hints.h>
+
+#include <protocols/eth.h>
+#include <protocols/ip.h>
+
+static int null_close(pktio_entry_t *pktio_entry ODP_UNUSED)
+{
+ return 0;
+}
+
+static int null_open(odp_pktio_t id ODP_UNUSED,
+ pktio_entry_t *pktio_entry ODP_UNUSED,
+ const char *devname, odp_pool_t pool ODP_UNUSED)
+{
+ if (strncmp(devname, "null:", 5) != 0)
+ return -1;
+ return 0;
+}
+
+static int null_recv(pktio_entry_t *pktio_entry ODP_UNUSED,
+ int index ODP_UNUSED, odp_packet_t pkt_table[] ODP_UNUSED,
+ int len ODP_UNUSED)
+{
+ return 0;
+}
+
+static int null_fd_set(pktio_entry_t *pktio_entry ODP_UNUSED,
+ int index ODP_UNUSED, fd_set *readfds)
+{
+ /* Somebody might wonder why readfds was not marked ODP_UNUSED.
+ * The reason is separately run checkpatch script complains that
+ * there must be spaces around '*', and if there are spaces, then
+ * the continuous integration build checkpatch script complains
+ * there must not be spaces around '*'. */
+ (void)readfds;
+ return 0;
+}
+
+static int null_recv_tmo(pktio_entry_t *pktio_entry ODP_UNUSED,
+ int index ODP_UNUSED,
+ odp_packet_t pkt_table[] ODP_UNUSED,
+ int num ODP_UNUSED, uint64_t usecs)
+{
+ struct timeval timeout;
+ int maxfd = -1;
+ fd_set readfds;
+
+ timeout.tv_sec = usecs / 1000 / 1000;
+ timeout.tv_usec = usecs % (1000 * 1000);
+ FD_ZERO(&readfds);
+
+ select(maxfd + 1, &readfds, NULL, NULL,
+ usecs == ODP_PKTIN_WAIT ? NULL : &timeout);
+
+ return 0;
+}
+
+static int null_recv_mq_tmo(pktio_entry_t *pktio_entry[] ODP_UNUSED,
+ int index[] ODP_UNUSED, int num_q ODP_UNUSED,
+ odp_packet_t pkt_table[] ODP_UNUSED,
+ int num ODP_UNUSED, unsigned *from ODP_UNUSED,
+ uint64_t usecs)
+{
+ struct timeval timeout;
+ int maxfd = -1;
+ fd_set readfds;
+
+ timeout.tv_sec = usecs / 1000 / 1000;
+ timeout.tv_usec = usecs % (1000 * 1000);
+
+ FD_ZERO(&readfds);
+
+ select(maxfd + 1, &readfds, NULL, NULL,
+ usecs == ODP_PKTIN_WAIT ? NULL : &timeout);
+
+ return 0;
+}
+
+static int null_send(pktio_entry_t *pktio_entry ODP_UNUSED,
+ int index ODP_UNUSED, const odp_packet_t pkt_table[],
+ int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++)
+ odp_packet_free(pkt_table[i]);
+
+ return len;
+}
+
+#define PKTIO_NULL_MTU (64 * 1024)
+
+static uint32_t null_mtu_get(pktio_entry_t *pktio_entry ODP_UNUSED)
+{
+ return PKTIO_NULL_MTU;
+}
+
+static const char null_mac[] = {0x02, 0xe9, 0x34, 0x80, 0x73, 0x05};
Comment:
@muvarov
At least it should have local bit set on. I was thinking about using part of
pktio name to generate mac.
> muvarov wrote
> I think other pktio have debug print that pktio is available on early init.
> Please keep the same style.
>> muvarov wrote
>> @lumag any generation thing is bad when you need to reproduce something. I
>> think all zeroes is good option here.
>>> muvarov wrote
>>> please clean up include files almost all of them are not needed for null.
>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>> At this point, since we're tagging Tiger Moth RC1 this week, all of
>>>> api-next is being merged into master, so this will be on that base.
>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>> Not at least on master; it isn't deprecated there. Because my changes
>>>>> weren't an API change, I based the code on master instead of basing it on
>>>>> api-next. Of course I'm willing to support this patch by implementing the
>>>>> non-deprecated API as well, if this is ever merged to master and then via
>>>>> master to api-next.
>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>> Ah, it is deprecated in api-next. My patch, however, is to master as it
>>>>>> has no API changes.
>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>> Shouldn't be too hard to support, just needs some space to store the
>>>>>>> flag. But will do.
>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>> In theory, a STATUS 0 would be considered an error here because the
>>>>>>>> expected status is -1 which is converted to 255.
>>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>>> Will fix.
>>>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>>>> The problem is that the pktio validation test case doesn't like an
>>>>>>>>>> interface where packets are not being received. It fails a number of
>>>>>>>>>> tests, and then for the pktio_test_recv_tmo it just hangs. With the
>>>>>>>>>> validation tests, an entirely different test for a quiescent
>>>>>>>>>> interface would be needed. Also, considering that this is testing a
>>>>>>>>>> particular driver ("null"), not a particular API, I'm not entirely
>>>>>>>>>> sure that API validation test would be the right location...
>>>>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>>>>> It isn't, at least in the master version on which this branch is
>>>>>>>>>>> based.
>>>>>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>>>>>> This is taken from the .pcap pktio, and incremented by one to make
>>>>>>>>>>>> it distinct from the .pcap MAC address. But agree, an internal API
>>>>>>>>>>>> to generate the MAC address would be great. It needs to be made
>>>>>>>>>>>> sure the multicast bit is not being set, then.
>>>>>>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>>>>>>> This will also be fixed in the next version.
>>>>>>>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>>>>>>>> I'll fix in the next version.
>>>>>>>>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>>>>>>>>> I'm not entirely sure about this... If the programmer wants to
>>>>>>>>>>>>>>> wait indefinitely, that's perhaps what we should do, which is
>>>>>>>>>>>>>>> indeed what is done now. The odp_pktio_stop() comment should
>>>>>>>>>>>>>>> perhaps be made to the PR #341. A good question is do we really
>>>>>>>>>>>>>>> need the empty readfds fd_set. Probably we could do without it.
>>>>>>>>>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>>>>>>>>>> Will do. This will result in a checkpatch warning then, but if
>>>>>>>>>>>>>>>> that is not an obstacle to a merge, I can do this.
>>>>>>>>>>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>>>>>>>>>>> Ok, the only reason I used this call was to increase code
>>>>>>>>>>>>>>>>> coverage. But is it deprecated? I grepped through the
>>>>>>>>>>>>>>>>> repository, finding to evidence that any API having the name
>>>>>>>>>>>>>>>>> mtu would be deprecated...
>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>> This should be supportable also. Just like it is done in
>>>>>>>>>>>>>>>>>> other software interfaces.
>>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>>> Maybe we should add internal API to generate random (or
>>>>>>>>>>>>>>>>>>> semi-random) local MAC-addresses with L bit being set?
>>>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>>>> Just use ODP_UNUSED and forget about checkpatch being
>>>>>>>>>>>>>>>>>>>> unoptimal here.
>>>>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>>>>> just exit $STATUS. This would allow underlying program to
>>>>>>>>>>>>>>>>>>>>> return 42 to mark the test as skipped
>>>>>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>>>>>> this should go under `if test_example`
>>>>>>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>>>>>>> I think this should be now in one of top-level
>>>>>>>>>>>>>>>>>>>>>>> .gitignore files.
>>>>>>>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>>>>>>>> This should probably go to a separate PR. And ideally
>>>>>>>>>>>>>>>>>>>>>>>> examples should not add much to code coverage on top
>>>>>>>>>>>>>>>>>>>>>>>> of tests, so you might want also to add code to api
>>>>>>>>>>>>>>>>>>>>>>>> testcases.
>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>> `odp_pktio_mtu()` is a deprecated API and should not
>>>>>>>>>>>>>>>>>>>>>>>>> be used in new code.
>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>> Where does this "magic number" come from? Why not
>>>>>>>>>>>>>>>>>>>>>>>>>> `00:00:00:00:00:00`?
>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>> This is a deprecated API. Not sure we need to
>>>>>>>>>>>>>>>>>>>>>>>>>>> include it for a new Pktio type.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>> The parameter should be `num`, not `len` here.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_packet_free_multi()` would be better here.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Same comments about `ODP_PKTIN_WAIT` here.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Shouldn't it be an error to try to receive from
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> a null device with `ODP_PKTIN_WAIT` since such
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> a call would never complete? Alternately one
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> could support such semantics and simply wait
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> indefinitely until `odp_pktio_stop()` is called
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to stop the interface.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Actually, looking at the semantics of
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_pktio_stop()` we're not precise about what
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> happens if there are pending
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `odp_pktio_recv_tmo()` call(s) on the interface
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> at the time `odp_pktio_stop()` is called.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Presumably these should be terminated,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> reporting no packets received when the stop
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> call is made.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> In this case it's probably better to be
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> consistent and use `(void)pktio_entry` and
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `(void)index` as well. Mixing this idiom with
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> `ODP_UNUSED` looks odd. It also needs no
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> explanation as both are acceptable.
https://github.com/Linaro/odp/pull/365#discussion_r158643513
updated_at 2017-12-25 13:19:53
