Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1235345 > unrolled thread
| Started by | Vivien Didelot <vivien.didelot@savoirfairelinux.com> |
|---|---|
| First post | 2015-09-29 18:10 +0200 |
| Last post | 2015-09-30 17:10 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v2 net-next 6/6] net: switchdev: extract struct switchdev_obj_* Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2015-09-29 18:10 +0200
Re: [PATCH v2 net-next 6/6] net: switchdev: extract struct switchdev_obj_* Jiri Pirko <jiri@resnulli.us> - 2015-09-30 12:40 +0200
Re: [PATCH v2 net-next 6/6] net: switchdev: extract struct switchdev_obj_* Scott Feldman <sfeldma@gmail.com> - 2015-09-30 17:00 +0200
Re: [PATCH v2 net-next 6/6] net: switchdev: extract struct switchdev_obj_* Jiri Pirko <jiri@resnulli.us> - 2015-09-30 17:10 +0200
| From | Vivien Didelot <vivien.didelot@savoirfairelinux.com> |
|---|---|
| Date | 2015-09-29 18:10 +0200 |
| Subject | [PATCH v2 net-next 6/6] net: switchdev: extract struct switchdev_obj_* |
| Message-ID | <qe5DA-2XB-15@gated-at.bofh.it> |
Now that switchdev and its drivers directly use specific switchdev_obj_*
structures, move them out of the switchdev_obj union and get rif of this
outer structure.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
include/net/switchdev.h | 53 ++++++++++++++++++++++++-------------------------
1 file changed, 26 insertions(+), 27 deletions(-)
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index bcadac3..e11425e 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -64,30 +64,29 @@ enum switchdev_obj_id {
SWITCHDEV_OBJ_PORT_FDB,
};
-struct switchdev_obj {
- enum switchdev_obj_id id;
- int (*cb)(struct switchdev_obj *obj);
- union {
- struct switchdev_obj_vlan { /* PORT_VLAN */
- u16 flags;
- u16 vid_begin;
- u16 vid_end;
- } vlan;
- struct switchdev_obj_ipv4_fib { /* IPV4_FIB */
- u32 dst;
- int dst_len;
- struct fib_info *fi;
- u8 tos;
- u8 type;
- u32 nlflags;
- u32 tb_id;
- } ipv4_fib;
- struct switchdev_obj_fdb { /* PORT_FDB */
- const unsigned char *addr;
- u16 vid;
- u16 ndm_state;
- } fdb;
- } u;
+/* SWITCHDEV_OBJ_PORT_VLAN */
+struct switchdev_obj_vlan {
+ u16 flags;
+ u16 vid_begin;
+ u16 vid_end;
+};
+
+/* SWITCHDEV_OBJ_IPV4_FIB */
+struct switchdev_obj_ipv4_fib {
+ u32 dst;
+ int dst_len;
+ struct fib_info *fi;
+ u8 tos;
+ u8 type;
+ u32 nlflags;
+ u32 tb_id;
+};
+
+/* SWITCHDEV_OBJ_PORT_FDB */
+struct switchdev_obj_fdb {
+ const unsigned char *addr;
+ u16 vid;
+ u16 ndm_state;
};
void switchdev_trans_item_enqueue(struct switchdev_trans *trans,
@@ -102,11 +101,11 @@ void *switchdev_trans_item_dequeue(struct switchdev_trans *trans);
*
* @switchdev_port_attr_set: Set a port attribute (see switchdev_attr).
*
- * @switchdev_port_obj_add: Add an object to port (see switchdev_obj).
+ * @switchdev_port_obj_add: Add an object to port (see switchdev_obj_*).
*
- * @switchdev_port_obj_del: Delete an object from port (see switchdev_obj).
+ * @switchdev_port_obj_del: Delete an object from port (see switchdev_obj_*).
*
- * @switchdev_port_obj_dump: Dump port objects (see switchdev_obj).
+ * @switchdev_port_obj_dump: Dump port objects (see switchdev_obj_*).
*/
struct switchdev_ops {
int (*switchdev_port_attr_get)(struct net_device *dev,
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Jiri Pirko <jiri@resnulli.us> |
|---|---|
| Date | 2015-09-30 12:40 +0200 |
| Subject | Re: [PATCH v2 net-next 6/6] net: switchdev: extract struct switchdev_obj_* |
| Message-ID | <qemXP-2nj-89@gated-at.bofh.it> |
| In reply to | #1235345 |
Tue, Sep 29, 2015 at 06:07:18PM CEST, vivien.didelot@savoirfairelinux.com wrote:
>Now that switchdev and its drivers directly use specific switchdev_obj_*
>structures, move them out of the switchdev_obj union and get rif of this
>outer structure.
>
>Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>---
> include/net/switchdev.h | 53 ++++++++++++++++++++++++-------------------------
> 1 file changed, 26 insertions(+), 27 deletions(-)
>
>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>index bcadac3..e11425e 100644
>--- a/include/net/switchdev.h
>+++ b/include/net/switchdev.h
>@@ -64,30 +64,29 @@ enum switchdev_obj_id {
> SWITCHDEV_OBJ_PORT_FDB,
> };
>
>-struct switchdev_obj {
>- enum switchdev_obj_id id;
>- int (*cb)(struct switchdev_obj *obj);
>- union {
>- struct switchdev_obj_vlan { /* PORT_VLAN */
>- u16 flags;
>- u16 vid_begin;
>- u16 vid_end;
>- } vlan;
>- struct switchdev_obj_ipv4_fib { /* IPV4_FIB */
>- u32 dst;
>- int dst_len;
>- struct fib_info *fi;
>- u8 tos;
>- u8 type;
>- u32 nlflags;
>- u32 tb_id;
>- } ipv4_fib;
>- struct switchdev_obj_fdb { /* PORT_FDB */
>- const unsigned char *addr;
>- u16 vid;
>- u16 ndm_state;
>- } fdb;
>- } u;
>+/* SWITCHDEV_OBJ_PORT_VLAN */
>+struct switchdev_obj_vlan {
>+ u16 flags;
>+ u16 vid_begin;
>+ u16 vid_end;
>+};
>+
>+/* SWITCHDEV_OBJ_IPV4_FIB */
>+struct switchdev_obj_ipv4_fib {
>+ u32 dst;
>+ int dst_len;
>+ struct fib_info *fi;
>+ u8 tos;
>+ u8 type;
>+ u32 nlflags;
>+ u32 tb_id;
>+};
>+
>+/* SWITCHDEV_OBJ_PORT_FDB */
>+struct switchdev_obj_fdb {
>+ const unsigned char *addr;
>+ u16 vid;
>+ u16 ndm_state;
> };
I don't like these structs being passed down as a "void *". I think that
we should have some "common" struct for these objects, event if it would
be empty and pass it down. "void *" does not look good at all, does not
tell the reader what that param is about. How about:
struct switchdev_obj {
};
struct switchdev_obj_vlan {
struct switchdev_obj obj;
u16 flags;
u16 vid_begin;
u16 vid_end;
};
#define SWITCHDEV_OBJ_VLAN(obj) \
container_of(obj, struct switchdev_obj_vlan, obj)
/* SWITCHDEV_OBJ_IPV4_FIB */
struct switchdev_obj_ipv4_fib {
struct switchdev_obj obj;
u32 dst;
int dst_len;
struct fib_info *fi;
u8 tos;
u8 type;
u32 nlflags;
u32 tb_id;
};
#define SWITCHDEV_OBJ_IPV4_FIB(obj) \
container_of(obj, struct switchdev_obj_ipv4_fib, obj)
/* SWITCHDEV_OBJ_PORT_FDB */
struct switchdev_obj_fdb {
struct switchdev_obj obj;
const unsigned char *addr;
u16 vid;
u16 ndm_state;
};
#define SWITCHDEV_OBJ_FDB(obj) \
container_of(obj, struct switchdev_obj_fdb, obj)
then pass struct switchdev_obj *obj down to drivers and in driver, get
original object by SWITCHDEV_OBJ_* ?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Scott Feldman <sfeldma@gmail.com> |
|---|---|
| Date | 2015-09-30 17:00 +0200 |
| Message-ID | <qer1o-8aa-19@gated-at.bofh.it> |
| In reply to | #1236117 |
On Wed, Sep 30, 2015 at 3:36 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Tue, Sep 29, 2015 at 06:07:18PM CEST, vivien.didelot@savoirfairelinux.com wrote:
>>Now that switchdev and its drivers directly use specific switchdev_obj_*
>>structures, move them out of the switchdev_obj union and get rif of this
>>outer structure.
>>
>>Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>>---
>> include/net/switchdev.h | 53 ++++++++++++++++++++++++-------------------------
>> 1 file changed, 26 insertions(+), 27 deletions(-)
>>
>>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>index bcadac3..e11425e 100644
>>--- a/include/net/switchdev.h
>>+++ b/include/net/switchdev.h
>>@@ -64,30 +64,29 @@ enum switchdev_obj_id {
>> SWITCHDEV_OBJ_PORT_FDB,
>> };
>>
>>-struct switchdev_obj {
>>- enum switchdev_obj_id id;
>>- int (*cb)(struct switchdev_obj *obj);
>>- union {
>>- struct switchdev_obj_vlan { /* PORT_VLAN */
>>- u16 flags;
>>- u16 vid_begin;
>>- u16 vid_end;
>>- } vlan;
>>- struct switchdev_obj_ipv4_fib { /* IPV4_FIB */
>>- u32 dst;
>>- int dst_len;
>>- struct fib_info *fi;
>>- u8 tos;
>>- u8 type;
>>- u32 nlflags;
>>- u32 tb_id;
>>- } ipv4_fib;
>>- struct switchdev_obj_fdb { /* PORT_FDB */
>>- const unsigned char *addr;
>>- u16 vid;
>>- u16 ndm_state;
>>- } fdb;
>>- } u;
>>+/* SWITCHDEV_OBJ_PORT_VLAN */
>>+struct switchdev_obj_vlan {
>>+ u16 flags;
>>+ u16 vid_begin;
>>+ u16 vid_end;
>>+};
>>+
>>+/* SWITCHDEV_OBJ_IPV4_FIB */
>>+struct switchdev_obj_ipv4_fib {
>>+ u32 dst;
>>+ int dst_len;
>>+ struct fib_info *fi;
>>+ u8 tos;
>>+ u8 type;
>>+ u32 nlflags;
>>+ u32 tb_id;
>>+};
>>+
>>+/* SWITCHDEV_OBJ_PORT_FDB */
>>+struct switchdev_obj_fdb {
>>+ const unsigned char *addr;
>>+ u16 vid;
>>+ u16 ndm_state;
>> };
>
>
> I don't like these structs being passed down as a "void *". I think that
> we should have some "common" struct for these objects, event if it would
> be empty and pass it down. "void *" does not look good at all, does not
> tell the reader what that param is about. How about:
>
> struct switchdev_obj {
> };
>
> struct switchdev_obj_vlan {
> struct switchdev_obj obj;
> u16 flags;
> u16 vid_begin;
> u16 vid_end;
> };
> #define SWITCHDEV_OBJ_VLAN(obj) \
> container_of(obj, struct switchdev_obj_vlan, obj)
>
> /* SWITCHDEV_OBJ_IPV4_FIB */
> struct switchdev_obj_ipv4_fib {
> struct switchdev_obj obj;
> u32 dst;
> int dst_len;
> struct fib_info *fi;
> u8 tos;
> u8 type;
> u32 nlflags;
> u32 tb_id;
> };
> #define SWITCHDEV_OBJ_IPV4_FIB(obj) \
> container_of(obj, struct switchdev_obj_ipv4_fib, obj)
>
> /* SWITCHDEV_OBJ_PORT_FDB */
> struct switchdev_obj_fdb {
> struct switchdev_obj obj;
> const unsigned char *addr;
> u16 vid;
> u16 ndm_state;
> };
> #define SWITCHDEV_OBJ_FDB(obj) \
> container_of(obj, struct switchdev_obj_fdb, obj)
>
>
> then pass struct switchdev_obj *obj down to drivers and in driver, get
> original object by SWITCHDEV_OBJ_* ?
Yes, I agree with Jiri, keep the struct switchdev_obj, use it within
each specific struct, and pass struct switchdev * in core funcs rather
than void *.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Jiri Pirko <jiri@resnulli.us> |
|---|---|
| Date | 2015-09-30 17:10 +0200 |
| Subject | Re: [PATCH v2 net-next 6/6] net: switchdev: extract struct switchdev_obj_* |
| Message-ID | <qerb4-9v-13@gated-at.bofh.it> |
| In reply to | #1236422 |
Wed, Sep 30, 2015 at 04:58:38PM CEST, sfeldma@gmail.com wrote:
>On Wed, Sep 30, 2015 at 3:36 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Tue, Sep 29, 2015 at 06:07:18PM CEST, vivien.didelot@savoirfairelinux.com wrote:
>>>Now that switchdev and its drivers directly use specific switchdev_obj_*
>>>structures, move them out of the switchdev_obj union and get rif of this
>>>outer structure.
>>>
>>>Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>>>---
>>> include/net/switchdev.h | 53 ++++++++++++++++++++++++-------------------------
>>> 1 file changed, 26 insertions(+), 27 deletions(-)
>>>
>>>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>>index bcadac3..e11425e 100644
>>>--- a/include/net/switchdev.h
>>>+++ b/include/net/switchdev.h
>>>@@ -64,30 +64,29 @@ enum switchdev_obj_id {
>>> SWITCHDEV_OBJ_PORT_FDB,
>>> };
>>>
>>>-struct switchdev_obj {
>>>- enum switchdev_obj_id id;
>>>- int (*cb)(struct switchdev_obj *obj);
>>>- union {
>>>- struct switchdev_obj_vlan { /* PORT_VLAN */
>>>- u16 flags;
>>>- u16 vid_begin;
>>>- u16 vid_end;
>>>- } vlan;
>>>- struct switchdev_obj_ipv4_fib { /* IPV4_FIB */
>>>- u32 dst;
>>>- int dst_len;
>>>- struct fib_info *fi;
>>>- u8 tos;
>>>- u8 type;
>>>- u32 nlflags;
>>>- u32 tb_id;
>>>- } ipv4_fib;
>>>- struct switchdev_obj_fdb { /* PORT_FDB */
>>>- const unsigned char *addr;
>>>- u16 vid;
>>>- u16 ndm_state;
>>>- } fdb;
>>>- } u;
>>>+/* SWITCHDEV_OBJ_PORT_VLAN */
>>>+struct switchdev_obj_vlan {
>>>+ u16 flags;
>>>+ u16 vid_begin;
>>>+ u16 vid_end;
>>>+};
>>>+
>>>+/* SWITCHDEV_OBJ_IPV4_FIB */
>>>+struct switchdev_obj_ipv4_fib {
>>>+ u32 dst;
>>>+ int dst_len;
>>>+ struct fib_info *fi;
>>>+ u8 tos;
>>>+ u8 type;
>>>+ u32 nlflags;
>>>+ u32 tb_id;
>>>+};
>>>+
>>>+/* SWITCHDEV_OBJ_PORT_FDB */
>>>+struct switchdev_obj_fdb {
>>>+ const unsigned char *addr;
>>>+ u16 vid;
>>>+ u16 ndm_state;
>>> };
>>
>>
>> I don't like these structs being passed down as a "void *". I think that
>> we should have some "common" struct for these objects, event if it would
>> be empty and pass it down. "void *" does not look good at all, does not
>> tell the reader what that param is about. How about:
>>
>> struct switchdev_obj {
>> };
>>
>> struct switchdev_obj_vlan {
>> struct switchdev_obj obj;
>> u16 flags;
>> u16 vid_begin;
>> u16 vid_end;
>> };
>> #define SWITCHDEV_OBJ_VLAN(obj) \
>> container_of(obj, struct switchdev_obj_vlan, obj)
>>
>> /* SWITCHDEV_OBJ_IPV4_FIB */
>> struct switchdev_obj_ipv4_fib {
>> struct switchdev_obj obj;
>> u32 dst;
>> int dst_len;
>> struct fib_info *fi;
>> u8 tos;
>> u8 type;
>> u32 nlflags;
>> u32 tb_id;
>> };
>> #define SWITCHDEV_OBJ_IPV4_FIB(obj) \
>> container_of(obj, struct switchdev_obj_ipv4_fib, obj)
>>
>> /* SWITCHDEV_OBJ_PORT_FDB */
>> struct switchdev_obj_fdb {
>> struct switchdev_obj obj;
>> const unsigned char *addr;
>> u16 vid;
>> u16 ndm_state;
>> };
>> #define SWITCHDEV_OBJ_FDB(obj) \
>> container_of(obj, struct switchdev_obj_fdb, obj)
>>
>>
>> then pass struct switchdev_obj *obj down to drivers and in driver, get
>> original object by SWITCHDEV_OBJ_* ?
>
>
>Yes, I agree with Jiri, keep the struct switchdev_obj, use it within
>each specific struct, and pass struct switchdev * in core funcs rather
>than void *.
I will cook-up a patch then. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web