diff mbox series

[v3,7/8] net/bonding: support checking valid bonding port ID

Message ID 20231008015041.1551165-8-chaoyong.he@corigine.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series Enhance the bond framework to support offload |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Chaoyong He Oct. 8, 2023, 1:50 a.m. UTC 
  From: Long Wu <long.wu@corigine.com>

Add API to support checking if the port id is a bonding
port id.

Signed-off-by: Long Wu <long.wu@corigine.com>
Reviewed-by: James Hershaw <james.hershaw@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/bonding/rte_eth_bond.h     | 13 +++++++++++++
 drivers/net/bonding/rte_eth_bond_api.c |  7 +++++++
 drivers/net/bonding/version.map        |  1 +
 3 files changed, 21 insertions(+)

Comments

lihuisong (C) Oct. 17, 2023, 8:33 a.m. UTC | #1 
Hi Chaoyong,

It is better to separate patch 3/8 and patch 6/8 from this series.

在 2023/10/8 9:50, Chaoyong He 写道:
> From: Long Wu <long.wu@corigine.com>
>
> Add API to support checking if the port id is a bonding
> port id.
>
> Signed-off-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: James Hershaw <james.hershaw@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> ---
>   drivers/net/bonding/rte_eth_bond.h     | 13 +++++++++++++
>   drivers/net/bonding/rte_eth_bond_api.c |  7 +++++++
>   drivers/net/bonding/version.map        |  1 +
>   3 files changed, 21 insertions(+)
>
> diff --git a/drivers/net/bonding/rte_eth_bond.h b/drivers/net/bonding/rte_eth_bond.h
> index 3f427b6bab..e8152a155f 100644
> --- a/drivers/net/bonding/rte_eth_bond.h
> +++ b/drivers/net/bonding/rte_eth_bond.h
> @@ -461,6 +461,19 @@ __rte_experimental
>   int
>   rte_eth_bond_hw_create_get(uint16_t bonding_port_id, uint16_t member_port_id);
>   
> +/**
> + * Check whether bonding port id is valid.
> + *
> + * @param bonding_port_id
> + *   Port ID of bonding device.
> + *
> + * @return
> + *   0 on success, negative value otherwise.
> + */
> +__rte_experimental
> +int
> +rte_eth_bond_valid_bonding_port_id(uint16_t bonding_port_id);
> +
>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
> index 0113dfdc16..80d71529cc 100644
> --- a/drivers/net/bonding/rte_eth_bond_api.c
> +++ b/drivers/net/bonding/rte_eth_bond_api.c
> @@ -1214,3 +1214,10 @@ rte_eth_bond_hw_create_get(uint16_t bonding_port_id, uint16_t member_port_id)
>   
>   	return member_dev->dev_ops->bond_hw_create_get(member_dev, bonding_dev);
>   }
> +
> +
> +int
> +rte_eth_bond_valid_bonding_port_id(uint16_t port_id)
> +{
> +	return valid_bonding_port_id(port_id);
> +}
> diff --git a/drivers/net/bonding/version.map b/drivers/net/bonding/version.map
> index 3cfff51269..bf5e50521e 100644
> --- a/drivers/net/bonding/version.map
> +++ b/drivers/net/bonding/version.map
> @@ -39,4 +39,5 @@ EXPERIMENTAL {
>   	rte_eth_bond_notify_member_flag_get;
>   	rte_eth_bond_notify_member_flag_set;
>   	rte_eth_bond_notify_members;
> +	rte_eth_bond_valid_bonding_port_id;
>   };
Chaoyong He Oct. 17, 2023, 9:25 a.m. UTC | #2 
> Hi Chaoyong,
> 
> It is better to separate patch 3/8 and patch 6/8 from this series.

The patch 3/8 is okay to separate, there is no problem.
But if patch 6/8 be a separate patch, this patch series will need depend on it.
I'm not sure if that is okay?

> 
> 在 2023/10/8 9:50, Chaoyong He 写道:
> > From: Long Wu <long.wu@corigine.com>
> >
> > Add API to support checking if the port id is a bonding port id.
> >
> > Signed-off-by: Long Wu <long.wu@corigine.com>
> > Reviewed-by: James Hershaw <james.hershaw@corigine.com>
> > Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> > ---
> >   drivers/net/bonding/rte_eth_bond.h     | 13 +++++++++++++
> >   drivers/net/bonding/rte_eth_bond_api.c |  7 +++++++
> >   drivers/net/bonding/version.map        |  1 +
> >   3 files changed, 21 insertions(+)
> >
> > diff --git a/drivers/net/bonding/rte_eth_bond.h
> > b/drivers/net/bonding/rte_eth_bond.h
> > index 3f427b6bab..e8152a155f 100644
> > --- a/drivers/net/bonding/rte_eth_bond.h
> > +++ b/drivers/net/bonding/rte_eth_bond.h
> > @@ -461,6 +461,19 @@ __rte_experimental
> >   int
> >   rte_eth_bond_hw_create_get(uint16_t bonding_port_id, uint16_t
> > member_port_id);
> >
> > +/**
> > + * Check whether bonding port id is valid.
> > + *
> > + * @param bonding_port_id
> > + *   Port ID of bonding device.
> > + *
> > + * @return
> > + *   0 on success, negative value otherwise.
> > + */
> > +__rte_experimental
> > +int
> > +rte_eth_bond_valid_bonding_port_id(uint16_t bonding_port_id);
> > +
> >   #ifdef __cplusplus
> >   }
> >   #endif
> > diff --git a/drivers/net/bonding/rte_eth_bond_api.c
> > b/drivers/net/bonding/rte_eth_bond_api.c
> > index 0113dfdc16..80d71529cc 100644
> > --- a/drivers/net/bonding/rte_eth_bond_api.c
> > +++ b/drivers/net/bonding/rte_eth_bond_api.c
> > @@ -1214,3 +1214,10 @@ rte_eth_bond_hw_create_get(uint16_t
> > bonding_port_id, uint16_t member_port_id)
> >
> >       return member_dev->dev_ops->bond_hw_create_get(member_dev,
> bonding_dev);
> >   }
> > +
> > +
> > +int
> > +rte_eth_bond_valid_bonding_port_id(uint16_t port_id) {
> > +     return valid_bonding_port_id(port_id); }
> > diff --git a/drivers/net/bonding/version.map
> > b/drivers/net/bonding/version.map index 3cfff51269..bf5e50521e 100644
> > --- a/drivers/net/bonding/version.map
> > +++ b/drivers/net/bonding/version.map
> > @@ -39,4 +39,5 @@ EXPERIMENTAL {
> >       rte_eth_bond_notify_member_flag_get;
> >       rte_eth_bond_notify_member_flag_set;
> >       rte_eth_bond_notify_members;
> > +     rte_eth_bond_valid_bonding_port_id;
> >   };
lihuisong (C) Oct. 17, 2023, 11:34 a.m. UTC | #3 
在 2023/10/17 17:25, Chaoyong He 写道:
>> Hi Chaoyong,
>>
>> It is better to separate patch 3/8 and patch 6/8 from this series.
> The patch 3/8 is okay to separate, there is no problem.
> But if patch 6/8 be a separate patch, this patch series will need depend on it.
> I'm not sure if that is okay?
I doesn't see the dependency of patch 7/8 and 8/8 on it.
If you remove the patches which isn't releated with the subject of this 
series, it is helpful for reviewing.
>
>> 在 2023/10/8 9:50, Chaoyong He 写道:
>>> From: Long Wu <long.wu@corigine.com>
>>>
>>> Add API to support checking if the port id is a bonding port id.
>>>
>>> Signed-off-by: Long Wu <long.wu@corigine.com>
>>> Reviewed-by: James Hershaw <james.hershaw@corigine.com>
>>> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
>>> ---
>>>    drivers/net/bonding/rte_eth_bond.h     | 13 +++++++++++++
>>>    drivers/net/bonding/rte_eth_bond_api.c |  7 +++++++
>>>    drivers/net/bonding/version.map        |  1 +
>>>    3 files changed, 21 insertions(+)
>>>
>>> diff --git a/drivers/net/bonding/rte_eth_bond.h
>>> b/drivers/net/bonding/rte_eth_bond.h
>>> index 3f427b6bab..e8152a155f 100644
>>> --- a/drivers/net/bonding/rte_eth_bond.h
>>> +++ b/drivers/net/bonding/rte_eth_bond.h
>>> @@ -461,6 +461,19 @@ __rte_experimental
>>>    int
>>>    rte_eth_bond_hw_create_get(uint16_t bonding_port_id, uint16_t
>>> member_port_id);
>>>
>>> +/**
>>> + * Check whether bonding port id is valid.
>>> + *
>>> + * @param bonding_port_id
>>> + *   Port ID of bonding device.
>>> + *
>>> + * @return
>>> + *   0 on success, negative value otherwise.
>>> + */
>>> +__rte_experimental
>>> +int
>>> +rte_eth_bond_valid_bonding_port_id(uint16_t bonding_port_id);
>>> +
>>>    #ifdef __cplusplus
>>>    }
>>>    #endif
>>> diff --git a/drivers/net/bonding/rte_eth_bond_api.c
>>> b/drivers/net/bonding/rte_eth_bond_api.c
>>> index 0113dfdc16..80d71529cc 100644
>>> --- a/drivers/net/bonding/rte_eth_bond_api.c
>>> +++ b/drivers/net/bonding/rte_eth_bond_api.c
>>> @@ -1214,3 +1214,10 @@ rte_eth_bond_hw_create_get(uint16_t
>>> bonding_port_id, uint16_t member_port_id)
>>>
>>>        return member_dev->dev_ops->bond_hw_create_get(member_dev,
>> bonding_dev);
>>>    }
>>> +
>>> +
>>> +int
>>> +rte_eth_bond_valid_bonding_port_id(uint16_t port_id) {
>>> +     return valid_bonding_port_id(port_id); }
>>> diff --git a/drivers/net/bonding/version.map
>>> b/drivers/net/bonding/version.map index 3cfff51269..bf5e50521e 100644
>>> --- a/drivers/net/bonding/version.map
>>> +++ b/drivers/net/bonding/version.map
>>> @@ -39,4 +39,5 @@ EXPERIMENTAL {
>>>        rte_eth_bond_notify_member_flag_get;
>>>        rte_eth_bond_notify_member_flag_set;
>>>        rte_eth_bond_notify_members;
>>> +     rte_eth_bond_valid_bonding_port_id;
>>>    };
Stephen Hemminger Oct. 17, 2023, 3:56 p.m. UTC | #4 
On Tue, 17 Oct 2023 16:33:26 +0800
"lihuisong (C)" <lihuisong@huawei.com> wrote:

> > +__rte_experimental
> > +int
> > +rte_eth_bond_valid_bonding_port_id(uint16_t bonding_port_id);

Ok, but my personal preference is to use shorter variable names
to avoid typos and overly long lines.

Maybe:

__rte_experimental
bool
rte_eth_bond_is_valid_port(uint16_t port_id)
Chaoyong He Oct. 18, 2023, 1:53 a.m. UTC | #5 
> 在 2023/10/17 17:25, Chaoyong He 写道:
> >> Hi Chaoyong,
> >>
> >> It is better to separate patch 3/8 and patch 6/8 from this series.
> > The patch 3/8 is okay to separate, there is no problem.
> > But if patch 6/8 be a separate patch, this patch series will need depend on it.
> > I'm not sure if that is okay?
> I doesn't see the dependency of patch 7/8 and 8/8 on it.
> If you remove the patches which isn't releated with the subject of this series, it
> is helpful for reviewing.

After discussing with Long Wu again, we confirmed that what you said is right.
Both patch 3/8 and 6/8 can be separated from this series, and we will do it in the next version.
Thanks.

> >
> >> 在 2023/10/8 9:50, Chaoyong He 写道:
> >>> From: Long Wu <long.wu@corigine.com>
> >>>
> >>> Add API to support checking if the port id is a bonding port id.
> >>>
> >>> Signed-off-by: Long Wu <long.wu@corigine.com>
> >>> Reviewed-by: James Hershaw <james.hershaw@corigine.com>
> >>> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> >>> ---
> >>>    drivers/net/bonding/rte_eth_bond.h     | 13 +++++++++++++
> >>>    drivers/net/bonding/rte_eth_bond_api.c |  7 +++++++
> >>>    drivers/net/bonding/version.map        |  1 +
> >>>    3 files changed, 21 insertions(+)
> >>>
> >>> diff --git a/drivers/net/bonding/rte_eth_bond.h
> >>> b/drivers/net/bonding/rte_eth_bond.h
> >>> index 3f427b6bab..e8152a155f 100644
> >>> --- a/drivers/net/bonding/rte_eth_bond.h
> >>> +++ b/drivers/net/bonding/rte_eth_bond.h
> >>> @@ -461,6 +461,19 @@ __rte_experimental
> >>>    int
> >>>    rte_eth_bond_hw_create_get(uint16_t bonding_port_id, uint16_t
> >>> member_port_id);
> >>>
> >>> +/**
> >>> + * Check whether bonding port id is valid.
> >>> + *
> >>> + * @param bonding_port_id
> >>> + *   Port ID of bonding device.
> >>> + *
> >>> + * @return
> >>> + *   0 on success, negative value otherwise.
> >>> + */
> >>> +__rte_experimental
> >>> +int
> >>> +rte_eth_bond_valid_bonding_port_id(uint16_t bonding_port_id);
> >>> +
> >>>    #ifdef __cplusplus
> >>>    }
> >>>    #endif
> >>> diff --git a/drivers/net/bonding/rte_eth_bond_api.c
> >>> b/drivers/net/bonding/rte_eth_bond_api.c
> >>> index 0113dfdc16..80d71529cc 100644
> >>> --- a/drivers/net/bonding/rte_eth_bond_api.c
> >>> +++ b/drivers/net/bonding/rte_eth_bond_api.c
> >>> @@ -1214,3 +1214,10 @@ rte_eth_bond_hw_create_get(uint16_t
> >>> bonding_port_id, uint16_t member_port_id)
> >>>
> >>>        return member_dev->dev_ops->bond_hw_create_get(member_dev,
> >> bonding_dev);
> >>>    }
> >>> +
> >>> +
> >>> +int
> >>> +rte_eth_bond_valid_bonding_port_id(uint16_t port_id) {
> >>> +     return valid_bonding_port_id(port_id); }
> >>> diff --git a/drivers/net/bonding/version.map
> >>> b/drivers/net/bonding/version.map index 3cfff51269..bf5e50521e
> >>> 100644
> >>> --- a/drivers/net/bonding/version.map
> >>> +++ b/drivers/net/bonding/version.map
> >>> @@ -39,4 +39,5 @@ EXPERIMENTAL {
> >>>        rte_eth_bond_notify_member_flag_get;
> >>>        rte_eth_bond_notify_member_flag_set;
> >>>        rte_eth_bond_notify_members;
> >>> +     rte_eth_bond_valid_bonding_port_id;
> >>>    };
diff mbox series

Patch

diff --git a/drivers/net/bonding/rte_eth_bond.h b/drivers/net/bonding/rte_eth_bond.h
index 3f427b6bab..e8152a155f 100644
--- a/drivers/net/bonding/rte_eth_bond.h
+++ b/drivers/net/bonding/rte_eth_bond.h
@@ -461,6 +461,19 @@  __rte_experimental
 int
 rte_eth_bond_hw_create_get(uint16_t bonding_port_id, uint16_t member_port_id);
 
+/**
+ * Check whether bonding port id is valid.
+ *
+ * @param bonding_port_id
+ *   Port ID of bonding device.
+ *
+ * @return
+ *   0 on success, negative value otherwise.
+ */
+__rte_experimental
+int
+rte_eth_bond_valid_bonding_port_id(uint16_t bonding_port_id);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index 0113dfdc16..80d71529cc 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -1214,3 +1214,10 @@  rte_eth_bond_hw_create_get(uint16_t bonding_port_id, uint16_t member_port_id)
 
 	return member_dev->dev_ops->bond_hw_create_get(member_dev, bonding_dev);
 }
+
+
+int
+rte_eth_bond_valid_bonding_port_id(uint16_t port_id)
+{
+	return valid_bonding_port_id(port_id);
+}
diff --git a/drivers/net/bonding/version.map b/drivers/net/bonding/version.map
index 3cfff51269..bf5e50521e 100644
--- a/drivers/net/bonding/version.map
+++ b/drivers/net/bonding/version.map
@@ -39,4 +39,5 @@  EXPERIMENTAL {
 	rte_eth_bond_notify_member_flag_get;
 	rte_eth_bond_notify_member_flag_set;
 	rte_eth_bond_notify_members;
+	rte_eth_bond_valid_bonding_port_id;
 };