muvarov replied on github web page:

platform/linux-generic/pktio/null.c
line 185
@@ -0,0 +1,207 @@
+/* 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_packet_null.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,
+                    const char *devname, odp_pool_t pool ODP_UNUSED)
+{
+       if (strncmp(devname, "null:", 5) != 0)
+               return -1;
+       pktio_entry->s.pkt_null.promisc = 0;
+       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 ODP_UNUSED)
+{
+       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 num)
+{
+       odp_packet_free_multi(pkt_table, num);
+
+       return num;
+}
+
+#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};
+
+static int null_mac_addr_get(pktio_entry_t *pktio_entry ODP_UNUSED,
+                            void *mac_addr)
+{
+       memcpy(mac_addr, null_mac, ETH_ALEN);
+       return ETH_ALEN;
+}
+
+static int null_promisc_mode_set(pktio_entry_t *pktio_entry, odp_bool_t enable)
+{
+       pktio_entry->s.pkt_null.promisc = !!enable;
+       return 0;
+}
+
+static int null_promisc_mode_get(pktio_entry_t *pktio_entry)
+{
+       return pktio_entry->s.pkt_null.promisc;
+}
+
+static int null_capability(pktio_entry_t *pktio_entry ODP_UNUSED,
+                          odp_pktio_capability_t *capa)
+{
+       memset(capa, 0, sizeof(odp_pktio_capability_t));
+
+       capa->max_input_queues  = PKTIO_MAX_QUEUES;
+       capa->max_output_queues = PKTIO_MAX_QUEUES;
+       capa->set_op.op.promisc_mode = 1;
+
+       odp_pktio_config_init(&capa->config);
+       capa->config.pktin.bit.ts_all = 1;
+       capa->config.pktin.bit.ts_ptp = 1;
+       return 0;
+}
+
+static int null_inqueues_config(pktio_entry_t *pktio_entry ODP_UNUSED,
+                               const odp_pktin_queue_param_t *p ODP_UNUSED)
+{
+       return 0;
+}
+
+static int null_outqueues_config(pktio_entry_t *pktio_entry ODP_UNUSED,
+                                const odp_pktout_queue_param_t *p ODP_UNUSED)
+{
+       return 0;
+}
+
+const pktio_if_ops_t null_pktio_ops = {
+       .name = "null",
+       .print = NULL,
+       .init_global = NULL,


Comment:
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_r158639355
updated_at 2017-12-25 11:31:53

Reply via email to