muvarov replied on github web page:
platform/linux-generic/pktio/null.c
line 34
@@ -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>
Comment:
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_r158639154
updated_at 2017-12-25 11:28:03
