On Tue, Dec 31, 2019 at 10:34 PM Stephen Hemminger
[off-list ref] wrote:

On Tue, 31 Dec 2019 16:53:15 +0530
gautamramk@gmail.com wrote:

quoted

From: "Mohit P. Tahiliani" <redacted>

This patch is a precursor for the addition of the Flow Queue Proportional
Integral Controller Enhanced (FQ-PIE) qdisc. The patch removes functions
and structures common to both PIE and FQ-PIE and moves it to the
header file pie.h

Signed-off-by: Mohit P. Tahiliani <redacted>
Signed-off-by: Sachin D. Patil <redacted>
Signed-off-by: V. Saicharan <redacted>
Signed-off-by: Mohit Bhasi <redacted>
Signed-off-by: Leslie Monis <redacted>
Signed-off-by: Gautam Ramakrishnan <redacted>
---
 include/net/pie.h   | 400 ++++++++++++++++++++++++++++++++++++++++++++
 net/sched/sch_pie.c | 386 ++----------------------------------------
 2 files changed, 415 insertions(+), 371 deletions(-)
 create mode 100644 include/net/pie.h

diff --git a/include/net/pie.h b/include/net/pie.h

Adding lots of static functions in a header file is not the way
to get code reuse in Linux kernel. It looks like you just did
large copy/paste from existing sch_pie.c to new header file.

You can use reuse data structures and small 'static inline' functions in a header file.
But putting code like drop_early in a header file is not best
practice.

You need to create a real kernel API for this kind of thing
by making a helper module which is reused by multiple places.

Hi Stephen,

Thanks for the feedback.

We did initially think of using EXPORT_SYMBOL to reuse large
functions like drop_early. However, we wanted to keep our
changes consistent with the existing Codel/FQ-Codel
implementation, and so we decided to move the functions
common to sch_pie.c and sch_fq_pie.c (the module we wish to
add) to pie.h, as done by codel and fq_codel.

Since you mentioned that this is not best practice, we're thinking
of simply exporting the required symbols from sch_pie.c and not
making an external helper module. We'll then create a module
dependency for sch_fq_pie.c on sch_pie.c. We'll still add
the pie.h header file to keep structures and small inline functions.

Would you recommend we go ahead with this?

Thanks,
Leslie