Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:
platform/linux-generic/pktio/pcap.c
line 160
@@ -423,25 +429,44 @@ static int pcapif_init_global(void)
return 0;
}
-const pktio_if_ops_t pcap_pktio_ops = {
- .name = "pcap",
- .print = NULL,
- .init_global = pcapif_init_global,
- .init_local = NULL,
+static pktio_ops_module_t pcap_pktio_ops = {
+ .base = {
+ .name = "pcap",
+ .init_local = NULL,
+ .term_local = NULL,
+ .init_global = pcapif_init_global,
+ .term_global = NULL,
+ },
.open = pcapif_init,
.close = pcapif_close,
+ .start = NULL,
+ .stop = NULL,
.stats = pcapif_stats,
.stats_reset = pcapif_stats_reset,
+ .pktin_ts_res = NULL,
+ .pktin_ts_from_ns = NULL,
.recv = pcapif_recv_pkt,
.send = pcapif_send_pkt,
.mtu_get = pcapif_mtu_get,
.promisc_mode_set = pcapif_promisc_mode_set,
.promisc_mode_get = pcapif_promisc_mode_get,
.mac_get = pcapif_mac_addr_get,
+ .link_status = NULL,
.capability = pcapif_capability,
- .pktin_ts_res = NULL,
- .pktin_ts_from_ns = NULL,
.config = NULL,
.input_queues_config = NULL,
.output_queues_config = NULL,
+ .print = NULL,
};
+
+ODP_MODULE_CONSTRUCTOR(pcap_pktio_ops)
+{
+ odp_module_constructor(&pcap_pktio_ops);
+
+ odp_subsystem_register_module(pktio_ops, &pcap_pktio_ops);
+}
+
+/* Temporary variable to enable link this module,
+ * will remove in Makefile scheme changes.
+ */
+int enable_link_pcap_pktio_ops = 0;
Comment:
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_r139839263
updated_at 2017-09-19 23:05:51
