He Yi(heyi-linaro) replied on github web page:
platform/linux-generic/pktio/tap.c
line 149
@@ -373,24 +372,44 @@ static int tap_capability(pktio_entry_t *pktio_entry
ODP_UNUSED,
return 0;
}
-const pktio_if_ops_t tap_pktio_ops = {
- .name = "tap",
- .print = NULL,
- .init_global = NULL,
- .init_local = NULL,
- .term = NULL,
+static pktio_ops_module_t tap_pktio_ops = {
+ .base = {
+ .name = "tap",
+ .init_local = NULL,
+ .term_local = NULL,
+ .init_global = NULL,
+ .term_global = NULL,
+ },
.open = tap_pktio_open,
.close = tap_pktio_close,
.start = NULL,
.stop = NULL,
+ .stats = NULL,
+ .stats_reset = NULL,
+ .pktin_ts_res = NULL,
+ .pktin_ts_from_ns = NULL,
.recv = tap_pktio_recv,
.send = tap_pktio_send,
.mtu_get = tap_mtu_get,
.promisc_mode_set = tap_promisc_mode_set,
.promisc_mode_get = tap_promisc_mode_get,
.mac_get = tap_mac_addr_get,
+ .link_status = NULL,
.capability = tap_capability,
- .pktin_ts_res = NULL,
- .pktin_ts_from_ns = NULL,
- .config = NULL
+ .config = NULL,
+ .input_queues_config = NULL,
+ .output_queues_config = NULL,
+ .print = NULL,
};
+
+ODP_MODULE_CONSTRUCTOR(tap_pktio_ops)
+{
+ odp_module_constructor(&tap_pktio_ops);
+
+ odp_subsystem_register_module(pktio_ops, &tap_pktio_ops);
+}
+
+/* Temporary variable to enable link this module,
+ * will remove in Makefile scheme changes.
+ */
+int enable_link_tap_pktio_ops = 0;
Comment:
Yes, created #184
> He Yi(heyi-linaro) wrote:
> Yes, created #184
>> He Yi(heyi-linaro) wrote:
>> Will fix all these.
>>> He Yi(heyi-linaro) wrote:
>>> Yes, created #184
>>>> He Yi(heyi-linaro) wrote:
>>>> Yes, created #184
>>>>> He Yi(heyi-linaro) wrote:
>>>>> Yes, created #184
>>>>>> He Yi(heyi-linaro) wrote:
>>>>>> Yes, will change it back to the original.
>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>> Yes, will fix all these.
>>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>>> Yes, created https://github.com/Linaro/odp/issues/184
>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>> Do not change interface names as part of this PR. Any name changes
>>>>>>>>> should be a separate PR where we can access application impact. In
>>>>>>>>> the case of DPDK devices this may be appropriate here, but not as
>>>>>>>>> part of this PR.
>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>> Same "to do" comment.
>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>> Same "to do" comment.
>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>> Again you've changed the interface name here from `socket_mmap` to
>>>>>>>>>>>> `socket mmap`. Keep the names unchanged as part of this PR to
>>>>>>>>>>>> avoid application impact.
>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>> Same "to do" comment.
>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>> Same "to do" comment.
>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>> This doesn't seem to go anywhere near the 80-char limit so
>>>>>>>>>>>>>>> should be fine as a single line.
>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>> Would be better to split it after `static int` if that's the
>>>>>>>>>>>>>>>> problem, or at a parameter boundary. Splitting after the open
>>>>>>>>>>>>>>>> paren looks odd. For example:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> ```
>>>>>>>>>>>>>>>> static int _pcapif_parse_devname(pktio_ops_pcap_data_t *pcap,
>>>>>>>>>>>>>>>> const char *devname)
>>>>>>>>>>>>>>>> ```
>>>>>>>>>>>>>>>> would be normal ODP style here.
>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>> Same "to do" comment here.
>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>> As noted, "to dos" should be tracked as issues. These need
>>>>>>>>>>>>>>>>>> not be separate issues, as the main purpose is simply to
>>>>>>>>>>>>>>>>>> remind us that pktio still has work to do.
>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>> Why are we changing this pktio name? This will cause
>>>>>>>>>>>>>>>>>>> existing applications that use the "loop" interface to need
>>>>>>>>>>>>>>>>>>> to change, which seems unnecessary. I'd keep this PR
>>>>>>>>>>>>>>>>>>> strictly to modularization without any application impact.
>>>>>>>>>>>>>>>>>>> If we want to change the pktio name that should be a
>>>>>>>>>>>>>>>>>>> separate PR.
>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>> Please open an issue so that we don't forget about this
>>>>>>>>>>>>>>>>>>>> "to do". We decided that this is a good use for GitHub
>>>>>>>>>>>>>>>>>>>> issues.
>>>>>>>>>>>>>>>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>>>>>>>>>>>>>>>> Run out of 80 characters?
>>>>>>>>>>>>>>>>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>> Run out of 80 characters?
>>>>>>>>>>>>>>>>>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>> Yes, I'm thinking how to achieve this. Can it be later
>>>>>>>>>>>>>>>>>>>>>>> patch improvements.
>>>>>>>>>>>>>>>>>>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>> This is temporary variables to make sure these modules
>>>>>>>>>>>>>>>>>>>>>>>> are linked into ODP library.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Because the Makefiles have not been re-organized to
>>>>>>>>>>>>>>>>>>>>>>>> build modules each as real independent module (.a or
>>>>>>>>>>>>>>>>>>>>>>>> .so) and either link them with --whole-archive or
>>>>>>>>>>>>>>>>>>>>>>>> --no-as-needed flags, this is necessary for current
>>>>>>>>>>>>>>>>>>>>>>>> Makefiles.
>>>>>>>>>>>>>>>>>>>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>> Yes, here we only use the ODP_SUBSYSTEM_API() macro
>>>>>>>>>>>>>>>>>>>>>>>>> to typedef prototype function pointers and leave the
>>>>>>>>>>>>>>>>>>>>>>>>> prototype declarations no defines.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> I suggest we accept this as is and consider how to
>>>>>>>>>>>>>>>>>>>>>>>>> handle this kind of tasks more properly in terms of
>>>>>>>>>>>>>>>>>>>>>>>>> autogen. as described in
>>>>>>>>>>>>>>>>>>>>>>>>> https://github.com/heyi-linaro/pure-interface/issues/6
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>> declaration fins in one line (there are others in
>>>>>>>>>>>>>>>>>>>>>>>>>> this file too)
>>>>>>>>>>>>>>>>>>>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>> indentation style
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Can we find a solution so these structures are
>>>>>>>>>>>>>>>>>>>>>>>>>>>> kept as const? If I get this correctly, apart from
>>>>>>>>>>>>>>>>>>>>>>>>>>>> the linked list embedded in the "base class",
>>>>>>>>>>>>>>>>>>>>>>>>>>>> there is nothing that should be mutable here,
>>>>>>>>>>>>>>>>>>>>>>>>>>>> right?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> how and where are these `enable_link_*` variables
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> used?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> How will this work with dynamically loadable
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> modules?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Isn't this going to declare an external function
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> prototype called odp_pktio_ops_open(...)?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> However the implementation won't exist, right?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I understand we need to define the function
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> types (odp_pktio_ops_open_t) for use in the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> structure below, but do we need to declare all
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> those function prototypes in this header file?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> GBalakrishna wrote
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> We have the working solution currently in
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> linux-dpdk. Question is, why we still want to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> have it in linux-generic ?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Yes, Krishna, as I replied in another email, I
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> suggest an approach that add workable
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> solutions first and after then remove obsolete
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> code.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> With PR#140 it looks likes that we removed
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> linux-generic dpdk pktio and lost dpdk pktio
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> at all, now wait for a new design which seems
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> still in discussion.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I'll comment these in your PR as a review
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> suggestion.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> GBalakrishna wrote
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I feel we don't need PKTIO_DPDK anylonger as
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> we now already have it from linux-dpdk. I
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> have sent a PR
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> [https://github.com/Linaro/odp/pull/140](url)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> with necessary updates.
https://github.com/Linaro/odp/pull/139#discussion_r139859431
updated_at 2017-09-20 01:52:13
