Juha-Matti Tilli(jmtilli) replied on github web page:

platform/linux-generic/pktio/null.c
@@ -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)


Comment:
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_r158597160
updated_at 2017-12-24 09:38:55

Reply via email to