* [PATCH net-next v3 0/2] propagate extack to vxlan_fdb_delete
@ 2022-04-24 12:09 Alaa Mohamed
2022-04-24 12:09 ` [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers Alaa Mohamed
2022-04-24 12:09 ` [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete Alaa Mohamed
0 siblings, 2 replies; 18+ messages in thread
From: Alaa Mohamed @ 2022-04-24 12:09 UTC (permalink / raw)
To: netdev
Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc, razor,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge, eng.alaamohamedsoliman.am
In order to propagate extack to vxlan_fdb_delete and vxlan_fdb_parse,
add extack to .ndo_fdb_del and edit all fdb del handelers
Alaa Mohamed (2):
rtnetlink: add extack support in fdb del handlers
net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
drivers/net/ethernet/intel/ice/ice_main.c | 4 +-
drivers/net/ethernet/mscc/ocelot_net.c | 4 +-
.../net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
drivers/net/macvlan.c | 2 +-
drivers/net/vxlan/vxlan_core.c | 38 +++++++++++++------
include/linux/netdevice.h | 2 +-
net/bridge/br_fdb.c | 2 +-
net/bridge/br_private.h | 2 +-
net/core/rtnetlink.c | 4 +-
9 files changed, 37 insertions(+), 23 deletions(-)
--
2.36.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
2022-04-24 12:09 [PATCH net-next v3 0/2] propagate extack to vxlan_fdb_delete Alaa Mohamed
@ 2022-04-24 12:09 ` Alaa Mohamed
2022-04-24 19:02 ` Nikolay Aleksandrov
2022-04-24 12:09 ` [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete Alaa Mohamed
1 sibling, 1 reply; 18+ messages in thread
From: Alaa Mohamed @ 2022-04-24 12:09 UTC (permalink / raw)
To: netdev
Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc, razor,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge, eng.alaamohamedsoliman.am
Add extack support to .ndo_fdb_del in netdevice.h and
all related methods.
Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
---
changes in V3:
fix errors reported by checkpatch.pl
---
drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
drivers/net/macvlan.c | 2 +-
drivers/net/vxlan/vxlan_core.c | 2 +-
include/linux/netdevice.h | 2 +-
net/bridge/br_fdb.c | 2 +-
net/bridge/br_private.h | 2 +-
net/core/rtnetlink.c | 4 ++--
9 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index d768925785ca..7b55d8d94803 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr __always_unused *tb[],
static int
ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
struct net_device *dev, const unsigned char *addr,
- __always_unused u16 vid)
+ __always_unused u16 vid, struct netlink_ext_ack *extack)
{
int err;
-
+
if (ndm->ndm_state & NUD_PERMANENT) {
netdev_err(dev, "FDB only supports static addresses\n");
return -EINVAL;
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 247bc105bdd2..e07c64e3159c 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev,
- const unsigned char *addr, u16 vid)
+ const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
{
struct ocelot_port_private *priv = netdev_priv(dev);
struct ocelot_port *ocelot_port = &priv->port;
struct ocelot *ocelot = ocelot_port->ocelot;
int port = priv->chip_port;
- return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge);
+ return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge, extack);
}
static int ocelot_port_fdb_dump(struct sk_buff *skb,
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index d320567b2cca..51fa23418f6a 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device *netdev, void *p)
static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *netdev,
- const unsigned char *addr, u16 vid)
+ const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
{
struct qlcnic_adapter *adapter = netdev_priv(netdev);
int err = -EOPNOTSUPP;
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 069e8824c264..ffd34d9f7049 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev,
- const unsigned char *addr, u16 vid)
+ const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
{
struct macvlan_dev *vlan = netdev_priv(dev);
int err = -EINVAL;
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index de97ff98d36e..cf2f60037340 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
/* Delete entry (via netlink) */
static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev,
- const unsigned char *addr, u16 vid)
+ const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
{
struct vxlan_dev *vxlan = netdev_priv(dev);
union vxlan_addr ip;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 28ea4f8269d4..d0d2a8f33c73 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1509,7 +1509,7 @@ struct net_device_ops {
struct nlattr *tb[],
struct net_device *dev,
const unsigned char *addr,
- u16 vid);
+ u16 vid, struct netlink_ext_ack *extack);
int (*ndo_fdb_dump)(struct sk_buff *skb,
struct netlink_callback *cb,
struct net_device *dev,
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 6ccda68bd473..5bfce2e9a553 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge *br,
/* Remove neighbor entry with RTM_DELNEIGH */
int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev,
- const unsigned char *addr, u16 vid)
+ const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
{
struct net_bridge_vlan_group *vg;
struct net_bridge_port *p = NULL;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 18ccc3d5d296..95348c1c9ce5 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
const unsigned char *addr, u16 vid, unsigned long flags);
int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
- struct net_device *dev, const unsigned char *addr, u16 vid);
+ struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev,
const unsigned char *addr, u16 vid, u16 nlh_flags,
struct netlink_ext_ack *extack);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 4041b3e2e8ec..99b30ae58a47 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
const struct net_device_ops *ops = br_dev->netdev_ops;
if (ops->ndo_fdb_del)
- err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
+ err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid, extack);
if (err)
goto out;
@@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
if (ndm->ndm_flags & NTF_SELF) {
if (dev->netdev_ops->ndo_fdb_del)
err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
- vid);
+ vid, extack);
else
err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
--
2.36.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
2022-04-24 12:09 [PATCH net-next v3 0/2] propagate extack to vxlan_fdb_delete Alaa Mohamed
2022-04-24 12:09 ` [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers Alaa Mohamed
@ 2022-04-24 12:09 ` Alaa Mohamed
2022-04-24 16:15 ` Julia Lawall
1 sibling, 1 reply; 18+ messages in thread
From: Alaa Mohamed @ 2022-04-24 12:09 UTC (permalink / raw)
To: netdev
Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc, razor,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge, eng.alaamohamedsoliman.am
Add extack to vxlan_fdb_delete and vxlan_fdb_parse
Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
---
changes in V2:
- fix spelling vxlan_fdb_delete
- add missing braces
- edit error message
---
changes in V3:
fix errors reported by checkpatch.pl
---
drivers/net/vxlan/vxlan_core.c | 36 +++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index cf2f60037340..4e1886655101 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -1129,19 +1129,23 @@ static void vxlan_fdb_dst_destroy(struct vxlan_dev *vxlan, struct vxlan_fdb *f,
static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
union vxlan_addr *ip, __be16 *port, __be32 *src_vni,
- __be32 *vni, u32 *ifindex, u32 *nhid)
+ __be32 *vni, u32 *ifindex, u32 *nhid, struct netlink_ext_ack *extack)
{
struct net *net = dev_net(vxlan->dev);
int err;
if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
- tb[NDA_PORT]))
- return -EINVAL;
+ tb[NDA_PORT])){
+ NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");
+ return -EINVAL;
+ }
if (tb[NDA_DST]) {
err = vxlan_nla_get_addr(ip, tb[NDA_DST]);
- if (err)
+ if (err){
+ NL_SET_ERR_MSG(extack, "Unsupported address family");
return err;
+ }
} else {
union vxlan_addr *remote = &vxlan->default_dst.remote_ip;
@@ -1157,24 +1161,30 @@ static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
}
if (tb[NDA_PORT]) {
- if (nla_len(tb[NDA_PORT]) != sizeof(__be16))
+ if (nla_len(tb[NDA_PORT]) != sizeof(__be16)){
+ NL_SET_ERR_MSG(extack, "Invalid vxlan port");
return -EINVAL;
+ }
*port = nla_get_be16(tb[NDA_PORT]);
} else {
*port = vxlan->cfg.dst_port;
}
if (tb[NDA_VNI]) {
- if (nla_len(tb[NDA_VNI]) != sizeof(u32))
+ if (nla_len(tb[NDA_VNI]) != sizeof(u32)){
+ NL_SET_ERR_MSG(extack, "Invalid vni");
return -EINVAL;
+ }
*vni = cpu_to_be32(nla_get_u32(tb[NDA_VNI]));
} else {
*vni = vxlan->default_dst.remote_vni;
}
if (tb[NDA_SRC_VNI]) {
- if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32))
+ if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32)){
+ NL_SET_ERR_MSG(extack, "Invalid src vni");
return -EINVAL;
+ }
*src_vni = cpu_to_be32(nla_get_u32(tb[NDA_SRC_VNI]));
} else {
*src_vni = vxlan->default_dst.remote_vni;
@@ -1183,12 +1193,16 @@ static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
if (tb[NDA_IFINDEX]) {
struct net_device *tdev;
- if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
+ if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32)){
+ NL_SET_ERR_MSG(extack, "Invalid ifindex");
return -EINVAL;
+ }
*ifindex = nla_get_u32(tb[NDA_IFINDEX]);
tdev = __dev_get_by_index(net, *ifindex);
- if (!tdev)
+ if (!tdev){
+ NL_SET_ERR_MSG(extack,"Device not found");
return -EADDRNOTAVAIL;
+ }
} else {
*ifindex = 0;
}
@@ -1226,7 +1240,7 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
return -EINVAL;
err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
- &nhid);
+ &nhid, extack);
if (err)
return err;
@@ -1291,7 +1305,7 @@ static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
int err;
err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
- &nhid);
+ &nhid, extack);
if (err)
return err;
--
2.36.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
2022-04-24 12:09 ` [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete Alaa Mohamed
@ 2022-04-24 16:15 ` Julia Lawall
2022-04-24 18:52 ` Alaa Mohamed
0 siblings, 1 reply; 18+ messages in thread
From: Julia Lawall @ 2022-04-24 16:15 UTC (permalink / raw)
To: Alaa Mohamed
Cc: netdev, outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc, razor,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
On Sun, 24 Apr 2022, Alaa Mohamed wrote:
> Add extack to vxlan_fdb_delete and vxlan_fdb_parse
It could be helpful to say more about what adding extack support involves.
>
> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
> ---
> changes in V2:
> - fix spelling vxlan_fdb_delete
> - add missing braces
> - edit error message
> ---
> changes in V3:
> fix errors reported by checkpatch.pl
But your changes would seem to also be introducing more checkpatch errors,
because the Linux kernel coding style puts a space before an { at the
start of an if branch.
julia
> ---
> drivers/net/vxlan/vxlan_core.c | 36 +++++++++++++++++++++++-----------
> 1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index cf2f60037340..4e1886655101 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -1129,19 +1129,23 @@ static void vxlan_fdb_dst_destroy(struct vxlan_dev *vxlan, struct vxlan_fdb *f,
>
> static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
> union vxlan_addr *ip, __be16 *port, __be32 *src_vni,
> - __be32 *vni, u32 *ifindex, u32 *nhid)
> + __be32 *vni, u32 *ifindex, u32 *nhid, struct netlink_ext_ack *extack)
> {
> struct net *net = dev_net(vxlan->dev);
> int err;
>
> if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
> - tb[NDA_PORT]))
> - return -EINVAL;
> + tb[NDA_PORT])){
> + NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");
> + return -EINVAL;
> + }
>
> if (tb[NDA_DST]) {
> err = vxlan_nla_get_addr(ip, tb[NDA_DST]);
> - if (err)
> + if (err){
> + NL_SET_ERR_MSG(extack, "Unsupported address family");
> return err;
> + }
> } else {
> union vxlan_addr *remote = &vxlan->default_dst.remote_ip;
>
> @@ -1157,24 +1161,30 @@ static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
> }
>
> if (tb[NDA_PORT]) {
> - if (nla_len(tb[NDA_PORT]) != sizeof(__be16))
> + if (nla_len(tb[NDA_PORT]) != sizeof(__be16)){
> + NL_SET_ERR_MSG(extack, "Invalid vxlan port");
> return -EINVAL;
> + }
> *port = nla_get_be16(tb[NDA_PORT]);
> } else {
> *port = vxlan->cfg.dst_port;
> }
>
> if (tb[NDA_VNI]) {
> - if (nla_len(tb[NDA_VNI]) != sizeof(u32))
> + if (nla_len(tb[NDA_VNI]) != sizeof(u32)){
> + NL_SET_ERR_MSG(extack, "Invalid vni");
> return -EINVAL;
> + }
> *vni = cpu_to_be32(nla_get_u32(tb[NDA_VNI]));
> } else {
> *vni = vxlan->default_dst.remote_vni;
> }
>
> if (tb[NDA_SRC_VNI]) {
> - if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32))
> + if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32)){
> + NL_SET_ERR_MSG(extack, "Invalid src vni");
> return -EINVAL;
> + }
> *src_vni = cpu_to_be32(nla_get_u32(tb[NDA_SRC_VNI]));
> } else {
> *src_vni = vxlan->default_dst.remote_vni;
> @@ -1183,12 +1193,16 @@ static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
> if (tb[NDA_IFINDEX]) {
> struct net_device *tdev;
>
> - if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
> + if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32)){
> + NL_SET_ERR_MSG(extack, "Invalid ifindex");
> return -EINVAL;
> + }
> *ifindex = nla_get_u32(tb[NDA_IFINDEX]);
> tdev = __dev_get_by_index(net, *ifindex);
> - if (!tdev)
> + if (!tdev){
> + NL_SET_ERR_MSG(extack,"Device not found");
> return -EADDRNOTAVAIL;
> + }
> } else {
> *ifindex = 0;
> }
> @@ -1226,7 +1240,7 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
> return -EINVAL;
>
> err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
> - &nhid);
> + &nhid, extack);
> if (err)
> return err;
>
> @@ -1291,7 +1305,7 @@ static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> int err;
>
> err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
> - &nhid);
> + &nhid, extack);
> if (err)
> return err;
>
> --
> 2.36.0
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
2022-04-24 16:15 ` Julia Lawall
@ 2022-04-24 18:52 ` Alaa Mohamed
2022-04-24 18:56 ` Julia Lawall
2022-04-24 19:03 ` Nikolay Aleksandrov
0 siblings, 2 replies; 18+ messages in thread
From: Alaa Mohamed @ 2022-04-24 18:52 UTC (permalink / raw)
To: Julia Lawall
Cc: netdev, outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc, razor,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
On ٢٤/٤/٢٠٢٢ ١٨:١٥, Julia Lawall wrote:
>
> On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>
>> Add extack to vxlan_fdb_delete and vxlan_fdb_parse
> It could be helpful to say more about what adding extack support involves.
>
>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>> ---
>> changes in V2:
>> - fix spelling vxlan_fdb_delete
>> - add missing braces
>> - edit error message
>> ---
>> changes in V3:
>> fix errors reported by checkpatch.pl
> But your changes would seem to also be introducing more checkpatch errors,
> because the Linux kernel coding style puts a space before an { at the
> start of an if branch.
I ran checkpatch script before submitting this patch and got no error
>
> julia
>
>> ---
>> drivers/net/vxlan/vxlan_core.c | 36 +++++++++++++++++++++++-----------
>> 1 file changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
>> index cf2f60037340..4e1886655101 100644
>> --- a/drivers/net/vxlan/vxlan_core.c
>> +++ b/drivers/net/vxlan/vxlan_core.c
>> @@ -1129,19 +1129,23 @@ static void vxlan_fdb_dst_destroy(struct vxlan_dev *vxlan, struct vxlan_fdb *f,
>>
>> static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
>> union vxlan_addr *ip, __be16 *port, __be32 *src_vni,
>> - __be32 *vni, u32 *ifindex, u32 *nhid)
>> + __be32 *vni, u32 *ifindex, u32 *nhid, struct netlink_ext_ack *extack)
>> {
>> struct net *net = dev_net(vxlan->dev);
>> int err;
>>
>> if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
>> - tb[NDA_PORT]))
>> - return -EINVAL;
>> + tb[NDA_PORT])){
>> + NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");
>> + return -EINVAL;
>> + }
>>
>> if (tb[NDA_DST]) {
>> err = vxlan_nla_get_addr(ip, tb[NDA_DST]);
>> - if (err)
>> + if (err){
>> + NL_SET_ERR_MSG(extack, "Unsupported address family");
>> return err;
>> + }
>> } else {
>> union vxlan_addr *remote = &vxlan->default_dst.remote_ip;
>>
>> @@ -1157,24 +1161,30 @@ static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
>> }
>>
>> if (tb[NDA_PORT]) {
>> - if (nla_len(tb[NDA_PORT]) != sizeof(__be16))
>> + if (nla_len(tb[NDA_PORT]) != sizeof(__be16)){
>> + NL_SET_ERR_MSG(extack, "Invalid vxlan port");
>> return -EINVAL;
>> + }
>> *port = nla_get_be16(tb[NDA_PORT]);
>> } else {
>> *port = vxlan->cfg.dst_port;
>> }
>>
>> if (tb[NDA_VNI]) {
>> - if (nla_len(tb[NDA_VNI]) != sizeof(u32))
>> + if (nla_len(tb[NDA_VNI]) != sizeof(u32)){
>> + NL_SET_ERR_MSG(extack, "Invalid vni");
>> return -EINVAL;
>> + }
>> *vni = cpu_to_be32(nla_get_u32(tb[NDA_VNI]));
>> } else {
>> *vni = vxlan->default_dst.remote_vni;
>> }
>>
>> if (tb[NDA_SRC_VNI]) {
>> - if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32))
>> + if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32)){
>> + NL_SET_ERR_MSG(extack, "Invalid src vni");
>> return -EINVAL;
>> + }
>> *src_vni = cpu_to_be32(nla_get_u32(tb[NDA_SRC_VNI]));
>> } else {
>> *src_vni = vxlan->default_dst.remote_vni;
>> @@ -1183,12 +1193,16 @@ static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
>> if (tb[NDA_IFINDEX]) {
>> struct net_device *tdev;
>>
>> - if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
>> + if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32)){
>> + NL_SET_ERR_MSG(extack, "Invalid ifindex");
>> return -EINVAL;
>> + }
>> *ifindex = nla_get_u32(tb[NDA_IFINDEX]);
>> tdev = __dev_get_by_index(net, *ifindex);
>> - if (!tdev)
>> + if (!tdev){
>> + NL_SET_ERR_MSG(extack,"Device not found");
>> return -EADDRNOTAVAIL;
>> + }
>> } else {
>> *ifindex = 0;
>> }
>> @@ -1226,7 +1240,7 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>> return -EINVAL;
>>
>> err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
>> - &nhid);
>> + &nhid, extack);
>> if (err)
>> return err;
>>
>> @@ -1291,7 +1305,7 @@ static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>> int err;
>>
>> err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
>> - &nhid);
>> + &nhid, extack);
>> if (err)
>> return err;
>>
>> --
>> 2.36.0
>>
>>
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
2022-04-24 18:52 ` Alaa Mohamed
@ 2022-04-24 18:56 ` Julia Lawall
2022-04-24 18:59 ` Alaa Mohamed
2022-04-24 19:03 ` Nikolay Aleksandrov
1 sibling, 1 reply; 18+ messages in thread
From: Julia Lawall @ 2022-04-24 18:56 UTC (permalink / raw)
To: Alaa Mohamed
Cc: Julia Lawall, netdev, outreachy, roopa, jdenham, sbrivio,
jesse.brandeburg, anthony.l.nguyen, davem, kuba, pabeni,
vladimir.oltean, claudiu.manoil, alexandre.belloni, shshaikh,
manishc, razor, intel-wired-lan, linux-kernel, UNGLinuxDriver,
GR-Linux-NIC-Dev, bridge
[-- Attachment #1: Type: text/plain, Size: 5072 bytes --]
On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>
> On ٢٤/٤/٢٠٢٢ ١٨:١٥, Julia Lawall wrote:
> >
> > On Sun, 24 Apr 2022, Alaa Mohamed wrote:
> >
> > > Add extack to vxlan_fdb_delete and vxlan_fdb_parse
> > It could be helpful to say more about what adding extack support involves.
> >
> > > Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
> > > ---
> > > changes in V2:
> > > - fix spelling vxlan_fdb_delete
> > > - add missing braces
> > > - edit error message
> > > ---
> > > changes in V3:
> > > fix errors reported by checkpatch.pl
> > But your changes would seem to also be introducing more checkpatch errors,
> > because the Linux kernel coding style puts a space before an { at the
> > start of an if branch.
> I ran checkpatch script before submitting this patch and got no error
OK, whether checkpatch complains or not doesn't really matter. The point
is that in the Linux kernel, one writes:
if (...) {
and not
if (...){
You can see other examples of ifs in the Linux kernel.
julia
> >
> > julia
> >
> > > ---
> > > drivers/net/vxlan/vxlan_core.c | 36 +++++++++++++++++++++++-----------
> > > 1 file changed, 25 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/net/vxlan/vxlan_core.c
> > > b/drivers/net/vxlan/vxlan_core.c
> > > index cf2f60037340..4e1886655101 100644
> > > --- a/drivers/net/vxlan/vxlan_core.c
> > > +++ b/drivers/net/vxlan/vxlan_core.c
> > > @@ -1129,19 +1129,23 @@ static void vxlan_fdb_dst_destroy(struct vxlan_dev
> > > *vxlan, struct vxlan_fdb *f,
> > >
> > > static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
> > > union vxlan_addr *ip, __be16 *port, __be32
> > > *src_vni,
> > > - __be32 *vni, u32 *ifindex, u32 *nhid)
> > > + __be32 *vni, u32 *ifindex, u32 *nhid, struct
> > > netlink_ext_ack *extack)
> > > {
> > > struct net *net = dev_net(vxlan->dev);
> > > int err;
> > >
> > > if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
> > > - tb[NDA_PORT]))
> > > - return -EINVAL;
> > > + tb[NDA_PORT])){
> > > + NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are
> > > mutually exclusive with NH_ID");
> > > + return -EINVAL;
> > > + }
> > >
> > > if (tb[NDA_DST]) {
> > > err = vxlan_nla_get_addr(ip, tb[NDA_DST]);
> > > - if (err)
> > > + if (err){
> > > + NL_SET_ERR_MSG(extack, "Unsupported address family");
> > > return err;
> > > + }
> > > } else {
> > > union vxlan_addr *remote = &vxlan->default_dst.remote_ip;
> > >
> > > @@ -1157,24 +1161,30 @@ static int vxlan_fdb_parse(struct nlattr *tb[],
> > > struct vxlan_dev *vxlan,
> > > }
> > >
> > > if (tb[NDA_PORT]) {
> > > - if (nla_len(tb[NDA_PORT]) != sizeof(__be16))
> > > + if (nla_len(tb[NDA_PORT]) != sizeof(__be16)){
> > > + NL_SET_ERR_MSG(extack, "Invalid vxlan port");
> > > return -EINVAL;
> > > + }
> > > *port = nla_get_be16(tb[NDA_PORT]);
> > > } else {
> > > *port = vxlan->cfg.dst_port;
> > > }
> > >
> > > if (tb[NDA_VNI]) {
> > > - if (nla_len(tb[NDA_VNI]) != sizeof(u32))
> > > + if (nla_len(tb[NDA_VNI]) != sizeof(u32)){
> > > + NL_SET_ERR_MSG(extack, "Invalid vni");
> > > return -EINVAL;
> > > + }
> > > *vni = cpu_to_be32(nla_get_u32(tb[NDA_VNI]));
> > > } else {
> > > *vni = vxlan->default_dst.remote_vni;
> > > }
> > >
> > > if (tb[NDA_SRC_VNI]) {
> > > - if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32))
> > > + if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32)){
> > > + NL_SET_ERR_MSG(extack, "Invalid src vni");
> > > return -EINVAL;
> > > + }
> > > *src_vni = cpu_to_be32(nla_get_u32(tb[NDA_SRC_VNI]));
> > > } else {
> > > *src_vni = vxlan->default_dst.remote_vni;
> > > @@ -1183,12 +1193,16 @@ static int vxlan_fdb_parse(struct nlattr *tb[],
> > > struct vxlan_dev *vxlan,
> > > if (tb[NDA_IFINDEX]) {
> > > struct net_device *tdev;
> > >
> > > - if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
> > > + if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32)){
> > > + NL_SET_ERR_MSG(extack, "Invalid ifindex");
> > > return -EINVAL;
> > > + }
> > > *ifindex = nla_get_u32(tb[NDA_IFINDEX]);
> > > tdev = __dev_get_by_index(net, *ifindex);
> > > - if (!tdev)
> > > + if (!tdev){
> > > + NL_SET_ERR_MSG(extack,"Device not found");
> > > return -EADDRNOTAVAIL;
> > > + }
> > > } else {
> > > *ifindex = 0;
> > > }
> > > @@ -1226,7 +1240,7 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct
> > > nlattr *tb[],
> > > return -EINVAL;
> > >
> > > err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
> > > - &nhid);
> > > + &nhid, extack);
> > > if (err)
> > > return err;
> > >
> > > @@ -1291,7 +1305,7 @@ static int vxlan_fdb_delete(struct ndmsg *ndm,
> > > struct nlattr *tb[],
> > > int err;
> > >
> > > err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
> > > - &nhid);
> > > + &nhid, extack);
> > > if (err)
> > > return err;
> > >
> > > --
> > > 2.36.0
> > >
> > >
> > >
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
2022-04-24 18:56 ` Julia Lawall
@ 2022-04-24 18:59 ` Alaa Mohamed
0 siblings, 0 replies; 18+ messages in thread
From: Alaa Mohamed @ 2022-04-24 18:59 UTC (permalink / raw)
To: Julia Lawall
Cc: netdev, outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc, razor,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
On ٢٤/٤/٢٠٢٢ ٢٠:٥٦, Julia Lawall wrote:
>
> On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>
>> On ٢٤/٤/٢٠٢٢ ١٨:١٥, Julia Lawall wrote:
>>> On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>>>
>>>> Add extack to vxlan_fdb_delete and vxlan_fdb_parse
>>> It could be helpful to say more about what adding extack support involves.
>>>
>>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>>> ---
>>>> changes in V2:
>>>> - fix spelling vxlan_fdb_delete
>>>> - add missing braces
>>>> - edit error message
>>>> ---
>>>> changes in V3:
>>>> fix errors reported by checkpatch.pl
>>> But your changes would seem to also be introducing more checkpatch errors,
>>> because the Linux kernel coding style puts a space before an { at the
>>> start of an if branch.
>> I ran checkpatch script before submitting this patch and got no error
> OK, whether checkpatch complains or not doesn't really matter. The point
> is that in the Linux kernel, one writes:
>
> if (...) {
>
> and not
>
> if (...){
>
> You can see other examples of ifs in the Linux kernel.
Yes, got it. I will fix it.
Thanks,
Alaa
>
> julia
>
>>> julia
>>>
>>>> ---
>>>> drivers/net/vxlan/vxlan_core.c | 36 +++++++++++++++++++++++-----------
>>>> 1 file changed, 25 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/net/vxlan/vxlan_core.c
>>>> b/drivers/net/vxlan/vxlan_core.c
>>>> index cf2f60037340..4e1886655101 100644
>>>> --- a/drivers/net/vxlan/vxlan_core.c
>>>> +++ b/drivers/net/vxlan/vxlan_core.c
>>>> @@ -1129,19 +1129,23 @@ static void vxlan_fdb_dst_destroy(struct vxlan_dev
>>>> *vxlan, struct vxlan_fdb *f,
>>>>
>>>> static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
>>>> union vxlan_addr *ip, __be16 *port, __be32
>>>> *src_vni,
>>>> - __be32 *vni, u32 *ifindex, u32 *nhid)
>>>> + __be32 *vni, u32 *ifindex, u32 *nhid, struct
>>>> netlink_ext_ack *extack)
>>>> {
>>>> struct net *net = dev_net(vxlan->dev);
>>>> int err;
>>>>
>>>> if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
>>>> - tb[NDA_PORT]))
>>>> - return -EINVAL;
>>>> + tb[NDA_PORT])){
>>>> + NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are
>>>> mutually exclusive with NH_ID");
>>>> + return -EINVAL;
>>>> + }
>>>>
>>>> if (tb[NDA_DST]) {
>>>> err = vxlan_nla_get_addr(ip, tb[NDA_DST]);
>>>> - if (err)
>>>> + if (err){
>>>> + NL_SET_ERR_MSG(extack, "Unsupported address family");
>>>> return err;
>>>> + }
>>>> } else {
>>>> union vxlan_addr *remote = &vxlan->default_dst.remote_ip;
>>>>
>>>> @@ -1157,24 +1161,30 @@ static int vxlan_fdb_parse(struct nlattr *tb[],
>>>> struct vxlan_dev *vxlan,
>>>> }
>>>>
>>>> if (tb[NDA_PORT]) {
>>>> - if (nla_len(tb[NDA_PORT]) != sizeof(__be16))
>>>> + if (nla_len(tb[NDA_PORT]) != sizeof(__be16)){
>>>> + NL_SET_ERR_MSG(extack, "Invalid vxlan port");
>>>> return -EINVAL;
>>>> + }
>>>> *port = nla_get_be16(tb[NDA_PORT]);
>>>> } else {
>>>> *port = vxlan->cfg.dst_port;
>>>> }
>>>>
>>>> if (tb[NDA_VNI]) {
>>>> - if (nla_len(tb[NDA_VNI]) != sizeof(u32))
>>>> + if (nla_len(tb[NDA_VNI]) != sizeof(u32)){
>>>> + NL_SET_ERR_MSG(extack, "Invalid vni");
>>>> return -EINVAL;
>>>> + }
>>>> *vni = cpu_to_be32(nla_get_u32(tb[NDA_VNI]));
>>>> } else {
>>>> *vni = vxlan->default_dst.remote_vni;
>>>> }
>>>>
>>>> if (tb[NDA_SRC_VNI]) {
>>>> - if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32))
>>>> + if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32)){
>>>> + NL_SET_ERR_MSG(extack, "Invalid src vni");
>>>> return -EINVAL;
>>>> + }
>>>> *src_vni = cpu_to_be32(nla_get_u32(tb[NDA_SRC_VNI]));
>>>> } else {
>>>> *src_vni = vxlan->default_dst.remote_vni;
>>>> @@ -1183,12 +1193,16 @@ static int vxlan_fdb_parse(struct nlattr *tb[],
>>>> struct vxlan_dev *vxlan,
>>>> if (tb[NDA_IFINDEX]) {
>>>> struct net_device *tdev;
>>>>
>>>> - if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
>>>> + if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32)){
>>>> + NL_SET_ERR_MSG(extack, "Invalid ifindex");
>>>> return -EINVAL;
>>>> + }
>>>> *ifindex = nla_get_u32(tb[NDA_IFINDEX]);
>>>> tdev = __dev_get_by_index(net, *ifindex);
>>>> - if (!tdev)
>>>> + if (!tdev){
>>>> + NL_SET_ERR_MSG(extack,"Device not found");
>>>> return -EADDRNOTAVAIL;
>>>> + }
>>>> } else {
>>>> *ifindex = 0;
>>>> }
>>>> @@ -1226,7 +1240,7 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct
>>>> nlattr *tb[],
>>>> return -EINVAL;
>>>>
>>>> err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
>>>> - &nhid);
>>>> + &nhid, extack);
>>>> if (err)
>>>> return err;
>>>>
>>>> @@ -1291,7 +1305,7 @@ static int vxlan_fdb_delete(struct ndmsg *ndm,
>>>> struct nlattr *tb[],
>>>> int err;
>>>>
>>>> err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
>>>> - &nhid);
>>>> + &nhid, extack);
>>>> if (err)
>>>> return err;
>>>>
>>>> --
>>>> 2.36.0
>>>>
>>>>
>>>>
> >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
2022-04-24 12:09 ` [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers Alaa Mohamed
@ 2022-04-24 19:02 ` Nikolay Aleksandrov
2022-04-24 19:49 ` Alaa Mohamed
0 siblings, 1 reply; 18+ messages in thread
From: Nikolay Aleksandrov @ 2022-04-24 19:02 UTC (permalink / raw)
To: Alaa Mohamed, netdev
Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
On 24/04/2022 15:09, Alaa Mohamed wrote:
> Add extack support to .ndo_fdb_del in netdevice.h and
> all related methods.
>
> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
> ---
> changes in V3:
> fix errors reported by checkpatch.pl
> ---
> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
> drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
> drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
> drivers/net/macvlan.c | 2 +-
> drivers/net/vxlan/vxlan_core.c | 2 +-
> include/linux/netdevice.h | 2 +-
> net/bridge/br_fdb.c | 2 +-
> net/bridge/br_private.h | 2 +-
> net/core/rtnetlink.c | 4 ++--
> 9 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index d768925785ca..7b55d8d94803 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr __always_unused *tb[],
> static int
> ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
> struct net_device *dev, const unsigned char *addr,
> - __always_unused u16 vid)
> + __always_unused u16 vid, struct netlink_ext_ack *extack)
> {
> int err;
> -
> +
What's changed here?
> if (ndm->ndm_state & NUD_PERMANENT) {
> netdev_err(dev, "FDB only supports static addresses\n");
> return -EINVAL;
> diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
> index 247bc105bdd2..e07c64e3159c 100644
> --- a/drivers/net/ethernet/mscc/ocelot_net.c
> +++ b/drivers/net/ethernet/mscc/ocelot_net.c
> @@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>
> static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> struct net_device *dev,
> - const unsigned char *addr, u16 vid)
> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
> {
> struct ocelot_port_private *priv = netdev_priv(dev);
> struct ocelot_port *ocelot_port = &priv->port;
> struct ocelot *ocelot = ocelot_port->ocelot;
> int port = priv->chip_port;
>
> - return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge);
> + return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge, extack);
> }
>
> static int ocelot_port_fdb_dump(struct sk_buff *skb,
> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> index d320567b2cca..51fa23418f6a 100644
> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> @@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device *netdev, void *p)
>
> static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> struct net_device *netdev,
> - const unsigned char *addr, u16 vid)
> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
> {
> struct qlcnic_adapter *adapter = netdev_priv(netdev);
> int err = -EOPNOTSUPP;
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index 069e8824c264..ffd34d9f7049 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>
> static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> struct net_device *dev,
> - const unsigned char *addr, u16 vid)
> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
> {
> struct macvlan_dev *vlan = netdev_priv(dev);
> int err = -EINVAL;
> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index de97ff98d36e..cf2f60037340 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
> /* Delete entry (via netlink) */
> static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> struct net_device *dev,
> - const unsigned char *addr, u16 vid)
> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
> {
> struct vxlan_dev *vxlan = netdev_priv(dev);
> union vxlan_addr ip;
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 28ea4f8269d4..d0d2a8f33c73 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1509,7 +1509,7 @@ struct net_device_ops {
> struct nlattr *tb[],
> struct net_device *dev,
> const unsigned char *addr,
> - u16 vid);
> + u16 vid, struct netlink_ext_ack *extack);
> int (*ndo_fdb_dump)(struct sk_buff *skb,
> struct netlink_callback *cb,
> struct net_device *dev,
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 6ccda68bd473..5bfce2e9a553 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge *br,
> /* Remove neighbor entry with RTM_DELNEIGH */
> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> struct net_device *dev,
> - const unsigned char *addr, u16 vid)
> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
> {
> struct net_bridge_vlan_group *vg;
> struct net_bridge_port *p = NULL;
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 18ccc3d5d296..95348c1c9ce5 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
> const unsigned char *addr, u16 vid, unsigned long flags);
>
> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> - struct net_device *dev, const unsigned char *addr, u16 vid);
> + struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
This is way too long (111 chars) and checkpatch should've complained about it.
WARNING: line length of 111 exceeds 100 columns
#234: FILE: net/bridge/br_private.h:782:
+ struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
> int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev,
> const unsigned char *addr, u16 vid, u16 nlh_flags,
> struct netlink_ext_ack *extack);
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 4041b3e2e8ec..99b30ae58a47 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
> const struct net_device_ops *ops = br_dev->netdev_ops;
>
> if (ops->ndo_fdb_del)
> - err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
> + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid, extack);
>
> if (err)
> goto out;
> @@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
> if (ndm->ndm_flags & NTF_SELF) {
> if (dev->netdev_ops->ndo_fdb_del)
> err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
> - vid);
> + vid, extack);
> else
> err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
>
> --
> 2.36.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
2022-04-24 18:52 ` Alaa Mohamed
2022-04-24 18:56 ` Julia Lawall
@ 2022-04-24 19:03 ` Nikolay Aleksandrov
2022-04-24 19:20 ` Alaa Mohamed
1 sibling, 1 reply; 18+ messages in thread
From: Nikolay Aleksandrov @ 2022-04-24 19:03 UTC (permalink / raw)
To: Alaa Mohamed, Julia Lawall
Cc: netdev, outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
On 24/04/2022 21:52, Alaa Mohamed wrote:
>
> On ٢٤/٤/٢٠٢٢ ١٨:١٥, Julia Lawall wrote:
>>
>> On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>>
>>> Add extack to vxlan_fdb_delete and vxlan_fdb_parse
>> It could be helpful to say more about what adding extack support involves.
>>
>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>> ---
>>> changes in V2:
>>> - fix spelling vxlan_fdb_delete
>>> - add missing braces
>>> - edit error message
>>> ---
>>> changes in V3:
>>> fix errors reported by checkpatch.pl
>> But your changes would seem to also be introducing more checkpatch errors,
>> because the Linux kernel coding style puts a space before an { at the
>> start of an if branch.
> I ran checkpatch script before submitting this patch and got no error
This is what I got:
WARNING: suspect code indent for conditional statements (8, 24)
#366: FILE: drivers/net/vxlan/vxlan_core.c:1137:
if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
[...]
+ NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");
WARNING: line length of 111 exceeds 100 columns
#370: FILE: drivers/net/vxlan/vxlan_core.c:1139:
+ NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");
ERROR: space required before the open brace '{'
#377: FILE: drivers/net/vxlan/vxlan_core.c:1145:
+ if (err){
ERROR: space required before the open brace '{'
#389: FILE: drivers/net/vxlan/vxlan_core.c:1164:
+ if (nla_len(tb[NDA_PORT]) != sizeof(__be16)){
ERROR: space required before the open brace '{'
#400: FILE: drivers/net/vxlan/vxlan_core.c:1174:
+ if (nla_len(tb[NDA_VNI]) != sizeof(u32)){
ERROR: space required before the open brace '{'
#411: FILE: drivers/net/vxlan/vxlan_core.c:1184:
+ if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32)){
ERROR: space required before the open brace '{'
#423: FILE: drivers/net/vxlan/vxlan_core.c:1196:
+ if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32)){
ERROR: space required before the open brace '{'
#430: FILE: drivers/net/vxlan/vxlan_core.c:1202:
+ if (!tdev){
ERROR: space required after that ',' (ctx:VxV)
#431: FILE: drivers/net/vxlan/vxlan_core.c:1203:
+ NL_SET_ERR_MSG(extack,"Device not found");
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
2022-04-24 19:03 ` Nikolay Aleksandrov
@ 2022-04-24 19:20 ` Alaa Mohamed
0 siblings, 0 replies; 18+ messages in thread
From: Alaa Mohamed @ 2022-04-24 19:20 UTC (permalink / raw)
To: Nikolay Aleksandrov, Julia Lawall
Cc: netdev, outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
On ٢٤/٤/٢٠٢٢ ٢١:٠٣, Nikolay Aleksandrov wrote:
> On 24/04/2022 21:52, Alaa Mohamed wrote:
>> On ٢٤/٤/٢٠٢٢ ١٨:١٥, Julia Lawall wrote:
>>> On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>>>
>>>> Add extack to vxlan_fdb_delete and vxlan_fdb_parse
>>> It could be helpful to say more about what adding extack support involves.
>>>
>>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>>> ---
>>>> changes in V2:
>>>> - fix spelling vxlan_fdb_delete
>>>> - add missing braces
>>>> - edit error message
>>>> ---
>>>> changes in V3:
>>>> fix errors reported by checkpatch.pl
>>> But your changes would seem to also be introducing more checkpatch errors,
>>> because the Linux kernel coding style puts a space before an { at the
>>> start of an if branch.
>> I ran checkpatch script before submitting this patch and got no error
> This is what I got:
> WARNING: suspect code indent for conditional statements (8, 24)
> #366: FILE: drivers/net/vxlan/vxlan_core.c:1137:
> if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
> [...]
> + NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");
>
> WARNING: line length of 111 exceeds 100 columns
> #370: FILE: drivers/net/vxlan/vxlan_core.c:1139:
> + NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");
>
> ERROR: space required before the open brace '{'
> #377: FILE: drivers/net/vxlan/vxlan_core.c:1145:
> + if (err){
>
> ERROR: space required before the open brace '{'
> #389: FILE: drivers/net/vxlan/vxlan_core.c:1164:
> + if (nla_len(tb[NDA_PORT]) != sizeof(__be16)){
>
> ERROR: space required before the open brace '{'
> #400: FILE: drivers/net/vxlan/vxlan_core.c:1174:
> + if (nla_len(tb[NDA_VNI]) != sizeof(u32)){
>
> ERROR: space required before the open brace '{'
> #411: FILE: drivers/net/vxlan/vxlan_core.c:1184:
> + if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32)){
>
> ERROR: space required before the open brace '{'
> #423: FILE: drivers/net/vxlan/vxlan_core.c:1196:
> + if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32)){
>
> ERROR: space required before the open brace '{'
> #430: FILE: drivers/net/vxlan/vxlan_core.c:1202:
> + if (!tdev){
>
> ERROR: space required after that ',' (ctx:VxV)
> #431: FILE: drivers/net/vxlan/vxlan_core.c:1203:
> + NL_SET_ERR_MSG(extack,"Device not found");
>
>
Thank you for sending that , but I don't know why I missed that when I
ran the script. Anyway, I fixed all these errors as Julia said.
Thanks again,
Alaa
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
2022-04-24 19:02 ` Nikolay Aleksandrov
@ 2022-04-24 19:49 ` Alaa Mohamed
2022-04-24 19:55 ` Nikolay Aleksandrov
2022-04-25 6:12 ` Julia Lawall
0 siblings, 2 replies; 18+ messages in thread
From: Alaa Mohamed @ 2022-04-24 19:49 UTC (permalink / raw)
To: Nikolay Aleksandrov, netdev
Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
> On 24/04/2022 15:09, Alaa Mohamed wrote:
>> Add extack support to .ndo_fdb_del in netdevice.h and
>> all related methods.
>>
>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>> ---
>> changes in V3:
>> fix errors reported by checkpatch.pl
>> ---
>> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
>> drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
>> drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
>> drivers/net/macvlan.c | 2 +-
>> drivers/net/vxlan/vxlan_core.c | 2 +-
>> include/linux/netdevice.h | 2 +-
>> net/bridge/br_fdb.c | 2 +-
>> net/bridge/br_private.h | 2 +-
>> net/core/rtnetlink.c | 4 ++--
>> 9 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
>> index d768925785ca..7b55d8d94803 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>> @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr __always_unused *tb[],
>> static int
>> ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
>> struct net_device *dev, const unsigned char *addr,
>> - __always_unused u16 vid)
>> + __always_unused u16 vid, struct netlink_ext_ack *extack)
>> {
>> int err;
>> -
>> +
> What's changed here?
In the previous version, I removed the blank line after "int err;" and
you said I shouldn't so I added blank line.
>
>> if (ndm->ndm_state & NUD_PERMANENT) {
>> netdev_err(dev, "FDB only supports static addresses\n");
>> return -EINVAL;
>> diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
>> index 247bc105bdd2..e07c64e3159c 100644
>> --- a/drivers/net/ethernet/mscc/ocelot_net.c
>> +++ b/drivers/net/ethernet/mscc/ocelot_net.c
>> @@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>>
>> static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>> struct net_device *dev,
>> - const unsigned char *addr, u16 vid)
>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>> {
>> struct ocelot_port_private *priv = netdev_priv(dev);
>> struct ocelot_port *ocelot_port = &priv->port;
>> struct ocelot *ocelot = ocelot_port->ocelot;
>> int port = priv->chip_port;
>>
>> - return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge);
>> + return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge, extack);
>> }
>>
>> static int ocelot_port_fdb_dump(struct sk_buff *skb,
>> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>> index d320567b2cca..51fa23418f6a 100644
>> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>> @@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device *netdev, void *p)
>>
>> static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>> struct net_device *netdev,
>> - const unsigned char *addr, u16 vid)
>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>> {
>> struct qlcnic_adapter *adapter = netdev_priv(netdev);
>> int err = -EOPNOTSUPP;
>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>> index 069e8824c264..ffd34d9f7049 100644
>> --- a/drivers/net/macvlan.c
>> +++ b/drivers/net/macvlan.c
>> @@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>>
>> static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>> struct net_device *dev,
>> - const unsigned char *addr, u16 vid)
>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>> {
>> struct macvlan_dev *vlan = netdev_priv(dev);
>> int err = -EINVAL;
>> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
>> index de97ff98d36e..cf2f60037340 100644
>> --- a/drivers/net/vxlan/vxlan_core.c
>> +++ b/drivers/net/vxlan/vxlan_core.c
>> @@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
>> /* Delete entry (via netlink) */
>> static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>> struct net_device *dev,
>> - const unsigned char *addr, u16 vid)
>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>> {
>> struct vxlan_dev *vxlan = netdev_priv(dev);
>> union vxlan_addr ip;
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 28ea4f8269d4..d0d2a8f33c73 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1509,7 +1509,7 @@ struct net_device_ops {
>> struct nlattr *tb[],
>> struct net_device *dev,
>> const unsigned char *addr,
>> - u16 vid);
>> + u16 vid, struct netlink_ext_ack *extack);
>> int (*ndo_fdb_dump)(struct sk_buff *skb,
>> struct netlink_callback *cb,
>> struct net_device *dev,
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 6ccda68bd473..5bfce2e9a553 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge *br,
>> /* Remove neighbor entry with RTM_DELNEIGH */
>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>> struct net_device *dev,
>> - const unsigned char *addr, u16 vid)
>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>> {
>> struct net_bridge_vlan_group *vg;
>> struct net_bridge_port *p = NULL;
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 18ccc3d5d296..95348c1c9ce5 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>> const unsigned char *addr, u16 vid, unsigned long flags);
>>
>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>> - struct net_device *dev, const unsigned char *addr, u16 vid);
>> + struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
> This is way too long (111 chars) and checkpatch should've complained about it.
> WARNING: line length of 111 exceeds 100 columns
> #234: FILE: net/bridge/br_private.h:782:
> + struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
I will fix it.
>
>> int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev,
>> const unsigned char *addr, u16 vid, u16 nlh_flags,
>> struct netlink_ext_ack *extack);
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index 4041b3e2e8ec..99b30ae58a47 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>> const struct net_device_ops *ops = br_dev->netdev_ops;
>>
>> if (ops->ndo_fdb_del)
>> - err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
>> + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid, extack);
>>
>> if (err)
>> goto out;
>> @@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>> if (ndm->ndm_flags & NTF_SELF) {
>> if (dev->netdev_ops->ndo_fdb_del)
>> err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
>> - vid);
>> + vid, extack);
>> else
>> err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
>>
>> --
>> 2.36.0
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
2022-04-24 19:49 ` Alaa Mohamed
@ 2022-04-24 19:55 ` Nikolay Aleksandrov
2022-04-24 21:09 ` Alaa Mohamed
2022-04-25 6:12 ` Julia Lawall
1 sibling, 1 reply; 18+ messages in thread
From: Nikolay Aleksandrov @ 2022-04-24 19:55 UTC (permalink / raw)
To: Alaa Mohamed, netdev
Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
On 24/04/2022 22:49, Alaa Mohamed wrote:
>
> On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
>> On 24/04/2022 15:09, Alaa Mohamed wrote:
>>> Add extack support to .ndo_fdb_del in netdevice.h and
>>> all related methods.
>>>
>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>> ---
>>> changes in V3:
>>> fix errors reported by checkpatch.pl
>>> ---
>>> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
>>> drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
>>> drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
>>> drivers/net/macvlan.c | 2 +-
>>> drivers/net/vxlan/vxlan_core.c | 2 +-
>>> include/linux/netdevice.h | 2 +-
>>> net/bridge/br_fdb.c | 2 +-
>>> net/bridge/br_private.h | 2 +-
>>> net/core/rtnetlink.c | 4 ++--
>>> 9 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
>>> index d768925785ca..7b55d8d94803 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>>> @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr __always_unused *tb[],
>>> static int
>>> ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
>>> struct net_device *dev, const unsigned char *addr,
>>> - __always_unused u16 vid)
>>> + __always_unused u16 vid, struct netlink_ext_ack *extack)
>>> {
>>> int err;
>>> -
>>> +
>> What's changed here?
>
> In the previous version, I removed the blank line after "int err;" and you said I shouldn't so I added blank line.
>
Yeah, my question is are you fixing a dos ending or something else?
The blank line is already there, what's wrong with it?
The point is it's not nice to mix style fixes and other changes, more so
if nothing is mentioned in the commit message.
>>
>>> if (ndm->ndm_state & NUD_PERMANENT) {
>>> netdev_err(dev, "FDB only supports static addresses\n");
>>> return -EINVAL;
>>> diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
>>> index 247bc105bdd2..e07c64e3159c 100644
>>> --- a/drivers/net/ethernet/mscc/ocelot_net.c
>>> +++ b/drivers/net/ethernet/mscc/ocelot_net.c
>>> @@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>>>
>>> static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>> struct net_device *dev,
>>> - const unsigned char *addr, u16 vid)
>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>> {
>>> struct ocelot_port_private *priv = netdev_priv(dev);
>>> struct ocelot_port *ocelot_port = &priv->port;
>>> struct ocelot *ocelot = ocelot_port->ocelot;
>>> int port = priv->chip_port;
>>>
>>> - return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge);
>>> + return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge, extack);
>>> }
>>>
>>> static int ocelot_port_fdb_dump(struct sk_buff *skb,
>>> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>> index d320567b2cca..51fa23418f6a 100644
>>> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>> @@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device *netdev, void *p)
>>>
>>> static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>> struct net_device *netdev,
>>> - const unsigned char *addr, u16 vid)
>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>> {
>>> struct qlcnic_adapter *adapter = netdev_priv(netdev);
>>> int err = -EOPNOTSUPP;
>>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>>> index 069e8824c264..ffd34d9f7049 100644
>>> --- a/drivers/net/macvlan.c
>>> +++ b/drivers/net/macvlan.c
>>> @@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>>>
>>> static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>> struct net_device *dev,
>>> - const unsigned char *addr, u16 vid)
>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>> {
>>> struct macvlan_dev *vlan = netdev_priv(dev);
>>> int err = -EINVAL;
>>> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
>>> index de97ff98d36e..cf2f60037340 100644
>>> --- a/drivers/net/vxlan/vxlan_core.c
>>> +++ b/drivers/net/vxlan/vxlan_core.c
>>> @@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
>>> /* Delete entry (via netlink) */
>>> static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>> struct net_device *dev,
>>> - const unsigned char *addr, u16 vid)
>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>> {
>>> struct vxlan_dev *vxlan = netdev_priv(dev);
>>> union vxlan_addr ip;
>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>> index 28ea4f8269d4..d0d2a8f33c73 100644
>>> --- a/include/linux/netdevice.h
>>> +++ b/include/linux/netdevice.h
>>> @@ -1509,7 +1509,7 @@ struct net_device_ops {
>>> struct nlattr *tb[],
>>> struct net_device *dev,
>>> const unsigned char *addr,
>>> - u16 vid);
>>> + u16 vid, struct netlink_ext_ack *extack);
>>> int (*ndo_fdb_dump)(struct sk_buff *skb,
>>> struct netlink_callback *cb,
>>> struct net_device *dev,
>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>> index 6ccda68bd473..5bfce2e9a553 100644
>>> --- a/net/bridge/br_fdb.c
>>> +++ b/net/bridge/br_fdb.c
>>> @@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge *br,
>>> /* Remove neighbor entry with RTM_DELNEIGH */
>>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>> struct net_device *dev,
>>> - const unsigned char *addr, u16 vid)
>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>> {
>>> struct net_bridge_vlan_group *vg;
>>> struct net_bridge_port *p = NULL;
>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>> index 18ccc3d5d296..95348c1c9ce5 100644
>>> --- a/net/bridge/br_private.h
>>> +++ b/net/bridge/br_private.h
>>> @@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>> const unsigned char *addr, u16 vid, unsigned long flags);
>>>
>>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>> - struct net_device *dev, const unsigned char *addr, u16 vid);
>>> + struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
>> This is way too long (111 chars) and checkpatch should've complained about it.
>> WARNING: line length of 111 exceeds 100 columns
>> #234: FILE: net/bridge/br_private.h:782:
>> + struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
>
> I will fix it.
>
>>
>>> int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev,
>>> const unsigned char *addr, u16 vid, u16 nlh_flags,
>>> struct netlink_ext_ack *extack);
>>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>>> index 4041b3e2e8ec..99b30ae58a47 100644
>>> --- a/net/core/rtnetlink.c
>>> +++ b/net/core/rtnetlink.c
>>> @@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>>> const struct net_device_ops *ops = br_dev->netdev_ops;
>>>
>>> if (ops->ndo_fdb_del)
>>> - err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
>>> + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid, extack);
>>>
>>> if (err)
>>> goto out;
>>> @@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>>> if (ndm->ndm_flags & NTF_SELF) {
>>> if (dev->netdev_ops->ndo_fdb_del)
>>> err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
>>> - vid);
>>> + vid, extack);
>>> else
>>> err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
>>>
>>> --
>>> 2.36.0
>>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
2022-04-24 19:55 ` Nikolay Aleksandrov
@ 2022-04-24 21:09 ` Alaa Mohamed
2022-04-24 21:52 ` Nikolay Aleksandrov
2022-04-25 6:11 ` Julia Lawall
0 siblings, 2 replies; 18+ messages in thread
From: Alaa Mohamed @ 2022-04-24 21:09 UTC (permalink / raw)
To: Nikolay Aleksandrov, netdev
Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
On ٢٤/٤/٢٠٢٢ ٢١:٥٥, Nikolay Aleksandrov wrote:
> On 24/04/2022 22:49, Alaa Mohamed wrote:
>> On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
>>> On 24/04/2022 15:09, Alaa Mohamed wrote:
>>>> Add extack support to .ndo_fdb_del in netdevice.h and
>>>> all related methods.
>>>>
>>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>>> ---
>>>> changes in V3:
>>>> fix errors reported by checkpatch.pl
>>>> ---
>>>> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
>>>> drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
>>>> drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
>>>> drivers/net/macvlan.c | 2 +-
>>>> drivers/net/vxlan/vxlan_core.c | 2 +-
>>>> include/linux/netdevice.h | 2 +-
>>>> net/bridge/br_fdb.c | 2 +-
>>>> net/bridge/br_private.h | 2 +-
>>>> net/core/rtnetlink.c | 4 ++--
>>>> 9 files changed, 12 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
>>>> index d768925785ca..7b55d8d94803 100644
>>>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>>>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>>>> @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr __always_unused *tb[],
>>>> static int
>>>> ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
>>>> struct net_device *dev, const unsigned char *addr,
>>>> - __always_unused u16 vid)
>>>> + __always_unused u16 vid, struct netlink_ext_ack *extack)
>>>> {
>>>> int err;
>>>> -
>>>> +
>>> What's changed here?
>> In the previous version, I removed the blank line after "int err;" and you said I shouldn't so I added blank line.
>>
> Yeah, my question is are you fixing a dos ending or something else?
> The blank line is already there, what's wrong with it?
No, I didn't.
>
> The point is it's not nice to mix style fixes and other changes, more so
> if nothing is mentioned in the commit message.
Got it, So, what should I do to fix it?
>>>> if (ndm->ndm_state & NUD_PERMANENT) {
>>>> netdev_err(dev, "FDB only supports static addresses\n");
>>>> return -EINVAL;
>>>> diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
>>>> index 247bc105bdd2..e07c64e3159c 100644
>>>> --- a/drivers/net/ethernet/mscc/ocelot_net.c
>>>> +++ b/drivers/net/ethernet/mscc/ocelot_net.c
>>>> @@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>>>>
>>>> static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>>> struct net_device *dev,
>>>> - const unsigned char *addr, u16 vid)
>>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>>> {
>>>> struct ocelot_port_private *priv = netdev_priv(dev);
>>>> struct ocelot_port *ocelot_port = &priv->port;
>>>> struct ocelot *ocelot = ocelot_port->ocelot;
>>>> int port = priv->chip_port;
>>>>
>>>> - return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge);
>>>> + return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge, extack);
>>>> }
>>>>
>>>> static int ocelot_port_fdb_dump(struct sk_buff *skb,
>>>> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>>> index d320567b2cca..51fa23418f6a 100644
>>>> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>>> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>>> @@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device *netdev, void *p)
>>>>
>>>> static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>>> struct net_device *netdev,
>>>> - const unsigned char *addr, u16 vid)
>>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>>> {
>>>> struct qlcnic_adapter *adapter = netdev_priv(netdev);
>>>> int err = -EOPNOTSUPP;
>>>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>>>> index 069e8824c264..ffd34d9f7049 100644
>>>> --- a/drivers/net/macvlan.c
>>>> +++ b/drivers/net/macvlan.c
>>>> @@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>>>>
>>>> static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>>> struct net_device *dev,
>>>> - const unsigned char *addr, u16 vid)
>>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>>> {
>>>> struct macvlan_dev *vlan = netdev_priv(dev);
>>>> int err = -EINVAL;
>>>> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
>>>> index de97ff98d36e..cf2f60037340 100644
>>>> --- a/drivers/net/vxlan/vxlan_core.c
>>>> +++ b/drivers/net/vxlan/vxlan_core.c
>>>> @@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
>>>> /* Delete entry (via netlink) */
>>>> static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>>> struct net_device *dev,
>>>> - const unsigned char *addr, u16 vid)
>>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>>> {
>>>> struct vxlan_dev *vxlan = netdev_priv(dev);
>>>> union vxlan_addr ip;
>>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>>> index 28ea4f8269d4..d0d2a8f33c73 100644
>>>> --- a/include/linux/netdevice.h
>>>> +++ b/include/linux/netdevice.h
>>>> @@ -1509,7 +1509,7 @@ struct net_device_ops {
>>>> struct nlattr *tb[],
>>>> struct net_device *dev,
>>>> const unsigned char *addr,
>>>> - u16 vid);
>>>> + u16 vid, struct netlink_ext_ack *extack);
>>>> int (*ndo_fdb_dump)(struct sk_buff *skb,
>>>> struct netlink_callback *cb,
>>>> struct net_device *dev,
>>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>>> index 6ccda68bd473..5bfce2e9a553 100644
>>>> --- a/net/bridge/br_fdb.c
>>>> +++ b/net/bridge/br_fdb.c
>>>> @@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge *br,
>>>> /* Remove neighbor entry with RTM_DELNEIGH */
>>>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>>> struct net_device *dev,
>>>> - const unsigned char *addr, u16 vid)
>>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>>> {
>>>> struct net_bridge_vlan_group *vg;
>>>> struct net_bridge_port *p = NULL;
>>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>>> index 18ccc3d5d296..95348c1c9ce5 100644
>>>> --- a/net/bridge/br_private.h
>>>> +++ b/net/bridge/br_private.h
>>>> @@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>>> const unsigned char *addr, u16 vid, unsigned long flags);
>>>>
>>>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>>> - struct net_device *dev, const unsigned char *addr, u16 vid);
>>>> + struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
>>> This is way too long (111 chars) and checkpatch should've complained about it.
>>> WARNING: line length of 111 exceeds 100 columns
>>> #234: FILE: net/bridge/br_private.h:782:
>>> + struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
>> I will fix it.
>>
>>>> int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev,
>>>> const unsigned char *addr, u16 vid, u16 nlh_flags,
>>>> struct netlink_ext_ack *extack);
>>>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>>>> index 4041b3e2e8ec..99b30ae58a47 100644
>>>> --- a/net/core/rtnetlink.c
>>>> +++ b/net/core/rtnetlink.c
>>>> @@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>>>> const struct net_device_ops *ops = br_dev->netdev_ops;
>>>>
>>>> if (ops->ndo_fdb_del)
>>>> - err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
>>>> + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid, extack);
>>>>
>>>> if (err)
>>>> goto out;
>>>> @@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>>>> if (ndm->ndm_flags & NTF_SELF) {
>>>> if (dev->netdev_ops->ndo_fdb_del)
>>>> err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
>>>> - vid);
>>>> + vid, extack);
>>>> else
>>>> err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
>>>>
>>>> --
>>>> 2.36.0
>>>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
2022-04-24 21:09 ` Alaa Mohamed
@ 2022-04-24 21:52 ` Nikolay Aleksandrov
2022-04-25 11:47 ` Alaa Mohamed
2022-04-25 6:11 ` Julia Lawall
1 sibling, 1 reply; 18+ messages in thread
From: Nikolay Aleksandrov @ 2022-04-24 21:52 UTC (permalink / raw)
To: Alaa Mohamed, netdev
Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
On 4/25/22 00:09, Alaa Mohamed wrote:
>
> On ٢٤/٤/٢٠٢٢ ٢١:٥٥, Nikolay Aleksandrov wrote:
>> On 24/04/2022 22:49, Alaa Mohamed wrote:
>>> On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
>>>> On 24/04/2022 15:09, Alaa Mohamed wrote:
>>>>> Add extack support to .ndo_fdb_del in netdevice.h and
>>>>> all related methods.
>>>>>
>>>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>>>> ---
>>>>> changes in V3:
>>>>> fix errors reported by checkpatch.pl
>>>>> ---
>>>>> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
>>>>> drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
>>>>> drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
>>>>> drivers/net/macvlan.c | 2 +-
>>>>> drivers/net/vxlan/vxlan_core.c | 2 +-
>>>>> include/linux/netdevice.h | 2 +-
>>>>> net/bridge/br_fdb.c | 2 +-
>>>>> net/bridge/br_private.h | 2 +-
>>>>> net/core/rtnetlink.c | 4 ++--
>>>>> 9 files changed, 12 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
>>>>> b/drivers/net/ethernet/intel/ice/ice_main.c
>>>>> index d768925785ca..7b55d8d94803 100644
>>>>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>>>>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>>>>> @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct
>>>>> nlattr __always_unused *tb[],
>>>>> static int
>>>>> ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
>>>>> struct net_device *dev, const unsigned char *addr,
>>>>> - __always_unused u16 vid)
>>>>> + __always_unused u16 vid, struct netlink_ext_ack *extack)
>>>>> {
>>>>> int err;
>>>>> -
>>>>> +
>>>> What's changed here?
>>> In the previous version, I removed the blank line after "int err;"
>>> and you said I shouldn't so I added blank line.
>>>
>> Yeah, my question is are you fixing a dos ending or something else?
>> The blank line is already there, what's wrong with it?
> No, I didn't.
>>
>> The point is it's not nice to mix style fixes and other changes, more so
>> if nothing is mentioned in the commit message.
> Got it, So, what should I do to fix it?
Don't change that line? I mean I'm even surprised this made it in the
patch. As I mentioned above, there is already a new line there so I'm
not sure how you're removing it and adding it again. :)
Cheers,
Nik
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
2022-04-24 21:09 ` Alaa Mohamed
2022-04-24 21:52 ` Nikolay Aleksandrov
@ 2022-04-25 6:11 ` Julia Lawall
2022-04-25 11:51 ` Alaa Mohamed
1 sibling, 1 reply; 18+ messages in thread
From: Julia Lawall @ 2022-04-25 6:11 UTC (permalink / raw)
To: Alaa Mohamed
Cc: Nikolay Aleksandrov, netdev, outreachy, roopa, jdenham, sbrivio,
jesse.brandeburg, anthony.l.nguyen, davem, kuba, pabeni,
vladimir.oltean, claudiu.manoil, alexandre.belloni, shshaikh,
manishc, intel-wired-lan, linux-kernel, UNGLinuxDriver,
GR-Linux-NIC-Dev, bridge
[-- Attachment #1: Type: text/plain, Size: 11714 bytes --]
On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>
> On ٢٤/٤/٢٠٢٢ ٢١:٥٥, Nikolay Aleksandrov wrote:
> > On 24/04/2022 22:49, Alaa Mohamed wrote:
> > > On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
> > > > On 24/04/2022 15:09, Alaa Mohamed wrote:
> > > > > Add extack support to .ndo_fdb_del in netdevice.h and
> > > > > all related methods.
> > > > >
> > > > > Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
> > > > > ---
> > > > > changes in V3:
> > > > > fix errors reported by checkpatch.pl
> > > > > ---
> > > > > drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
> > > > > drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
> > > > > drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
> > > > > drivers/net/macvlan.c | 2 +-
> > > > > drivers/net/vxlan/vxlan_core.c | 2 +-
> > > > > include/linux/netdevice.h | 2 +-
> > > > > net/bridge/br_fdb.c | 2 +-
> > > > > net/bridge/br_private.h | 2 +-
> > > > > net/core/rtnetlink.c | 4 ++--
> > > > > 9 files changed, 12 insertions(+), 12 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> > > > > b/drivers/net/ethernet/intel/ice/ice_main.c
> > > > > index d768925785ca..7b55d8d94803 100644
> > > > > --- a/drivers/net/ethernet/intel/ice/ice_main.c
> > > > > +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> > > > > @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr
> > > > > __always_unused *tb[],
> > > > > static int
> > > > > ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
> > > > > struct net_device *dev, const unsigned char *addr,
> > > > > - __always_unused u16 vid)
> > > > > + __always_unused u16 vid, struct netlink_ext_ack *extack)
> > > > > {
> > > > > int err;
> > > > > -
> > > > > +
> > > > What's changed here?
> > > In the previous version, I removed the blank line after "int err;" and you
> > > said I shouldn't so I added blank line.
> > >
> > Yeah, my question is are you fixing a dos ending or something else?
> > The blank line is already there, what's wrong with it?
> No, I didn't.
OK, so what is the answer to the question about what changed? It looks
like you remove a blank line and then add it back. But that should not
show up as a difference when you generate the patch.
When you answer a comment, please put a blank line before and after your
answer. Otherwise it can be hard to see your answer when it is in the
middle of a larger patch.
> >
> > The point is it's not nice to mix style fixes and other changes, more so
> > if nothing is mentioned in the commit message.
> Got it, So, what should I do to fix it?
A series? But it is not clear that any change is needed here at all.
julia
> > > > > if (ndm->ndm_state & NUD_PERMANENT) {
> > > > > netdev_err(dev, "FDB only supports static addresses\n");
> > > > > return -EINVAL;
> > > > > diff --git a/drivers/net/ethernet/mscc/ocelot_net.c
> > > > > b/drivers/net/ethernet/mscc/ocelot_net.c
> > > > > index 247bc105bdd2..e07c64e3159c 100644
> > > > > --- a/drivers/net/ethernet/mscc/ocelot_net.c
> > > > > +++ b/drivers/net/ethernet/mscc/ocelot_net.c
> > > > > @@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg
> > > > > *ndm, struct nlattr *tb[],
> > > > >
> > > > > static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr
> > > > > *tb[],
> > > > > struct net_device *dev,
> > > > > - const unsigned char *addr, u16 vid)
> > > > > + const unsigned char *addr, u16 vid, struct
> > > > > netlink_ext_ack *extack)
> > > > > {
> > > > > struct ocelot_port_private *priv = netdev_priv(dev);
> > > > > struct ocelot_port *ocelot_port = &priv->port;
> > > > > struct ocelot *ocelot = ocelot_port->ocelot;
> > > > > int port = priv->chip_port;
> > > > >
> > > > > - return ocelot_fdb_del(ocelot, port, addr, vid,
> > > > > ocelot_port->bridge);
> > > > > + return ocelot_fdb_del(ocelot, port, addr, vid,
> > > > > ocelot_port->bridge, extack);
> > > > > }
> > > > >
> > > > > static int ocelot_port_fdb_dump(struct sk_buff *skb,
> > > > > diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > > > b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > > > index d320567b2cca..51fa23418f6a 100644
> > > > > --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > > > +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > > > @@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device
> > > > > *netdev, void *p)
> > > > >
> > > > > static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> > > > > struct net_device *netdev,
> > > > > - const unsigned char *addr, u16 vid)
> > > > > + const unsigned char *addr, u16 vid, struct
> > > > > netlink_ext_ack *extack)
> > > > > {
> > > > > struct qlcnic_adapter *adapter = netdev_priv(netdev);
> > > > > int err = -EOPNOTSUPP;
> > > > > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> > > > > index 069e8824c264..ffd34d9f7049 100644
> > > > > --- a/drivers/net/macvlan.c
> > > > > +++ b/drivers/net/macvlan.c
> > > > > @@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm,
> > > > > struct nlattr *tb[],
> > > > >
> > > > > static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> > > > > struct net_device *dev,
> > > > > - const unsigned char *addr, u16 vid)
> > > > > + const unsigned char *addr, u16 vid, struct
> > > > > netlink_ext_ack *extack)
> > > > > {
> > > > > struct macvlan_dev *vlan = netdev_priv(dev);
> > > > > int err = -EINVAL;
> > > > > diff --git a/drivers/net/vxlan/vxlan_core.c
> > > > > b/drivers/net/vxlan/vxlan_core.c
> > > > > index de97ff98d36e..cf2f60037340 100644
> > > > > --- a/drivers/net/vxlan/vxlan_core.c
> > > > > +++ b/drivers/net/vxlan/vxlan_core.c
> > > > > @@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
> > > > > /* Delete entry (via netlink) */
> > > > > static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> > > > > struct net_device *dev,
> > > > > - const unsigned char *addr, u16 vid)
> > > > > + const unsigned char *addr, u16 vid, struct
> > > > > netlink_ext_ack *extack)
> > > > > {
> > > > > struct vxlan_dev *vxlan = netdev_priv(dev);
> > > > > union vxlan_addr ip;
> > > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > > > index 28ea4f8269d4..d0d2a8f33c73 100644
> > > > > --- a/include/linux/netdevice.h
> > > > > +++ b/include/linux/netdevice.h
> > > > > @@ -1509,7 +1509,7 @@ struct net_device_ops {
> > > > > struct nlattr *tb[],
> > > > > struct net_device *dev,
> > > > > const unsigned char *addr,
> > > > > - u16 vid);
> > > > > + u16 vid, struct netlink_ext_ack *extack);
> > > > > int (*ndo_fdb_dump)(struct sk_buff *skb,
> > > > > struct netlink_callback *cb,
> > > > > struct net_device *dev,
> > > > > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> > > > > index 6ccda68bd473..5bfce2e9a553 100644
> > > > > --- a/net/bridge/br_fdb.c
> > > > > +++ b/net/bridge/br_fdb.c
> > > > > @@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge
> > > > > *br,
> > > > > /* Remove neighbor entry with RTM_DELNEIGH */
> > > > > int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> > > > > struct net_device *dev,
> > > > > - const unsigned char *addr, u16 vid)
> > > > > + const unsigned char *addr, u16 vid, struct netlink_ext_ack
> > > > > *extack)
> > > > > {
> > > > > struct net_bridge_vlan_group *vg;
> > > > > struct net_bridge_port *p = NULL;
> > > > > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> > > > > index 18ccc3d5d296..95348c1c9ce5 100644
> > > > > --- a/net/bridge/br_private.h
> > > > > +++ b/net/bridge/br_private.h
> > > > > @@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct
> > > > > net_bridge_port *source,
> > > > > const unsigned char *addr, u16 vid, unsigned long
> > > > > flags);
> > > > >
> > > > > int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> > > > > - struct net_device *dev, const unsigned char *addr, u16
> > > > > vid);
> > > > > + struct net_device *dev, const unsigned char *addr, u16 vid,
> > > > > struct netlink_ext_ack *extack);
> > > > This is way too long (111 chars) and checkpatch should've complained
> > > > about it.
> > > > WARNING: line length of 111 exceeds 100 columns
> > > > #234: FILE: net/bridge/br_private.h:782:
> > > > + struct net_device *dev, const unsigned char *addr, u16 vid,
> > > > struct netlink_ext_ack *extack);
> > > I will fix it.
> > >
> > > > > int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct
> > > > > net_device *dev,
> > > > > const unsigned char *addr, u16 vid, u16 nlh_flags,
> > > > > struct netlink_ext_ack *extack);
> > > > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > > > > index 4041b3e2e8ec..99b30ae58a47 100644
> > > > > --- a/net/core/rtnetlink.c
> > > > > +++ b/net/core/rtnetlink.c
> > > > > @@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb,
> > > > > struct nlmsghdr *nlh,
> > > > > const struct net_device_ops *ops = br_dev->netdev_ops;
> > > > >
> > > > > if (ops->ndo_fdb_del)
> > > > > - err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
> > > > > + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid, extack);
> > > > >
> > > > > if (err)
> > > > > goto out;
> > > > > @@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb,
> > > > > struct nlmsghdr *nlh,
> > > > > if (ndm->ndm_flags & NTF_SELF) {
> > > > > if (dev->netdev_ops->ndo_fdb_del)
> > > > > err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
> > > > > - vid);
> > > > > + vid, extack);
> > > > > else
> > > > > err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
> > > > >
> > > > > --
> > > > > 2.36.0
> > > > >
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
2022-04-24 19:49 ` Alaa Mohamed
2022-04-24 19:55 ` Nikolay Aleksandrov
@ 2022-04-25 6:12 ` Julia Lawall
1 sibling, 0 replies; 18+ messages in thread
From: Julia Lawall @ 2022-04-25 6:12 UTC (permalink / raw)
To: Alaa Mohamed
Cc: Nikolay Aleksandrov, netdev, outreachy, roopa, jdenham, sbrivio,
jesse.brandeburg, anthony.l.nguyen, davem, kuba, pabeni,
vladimir.oltean, claudiu.manoil, alexandre.belloni, shshaikh,
manishc, intel-wired-lan, linux-kernel, UNGLinuxDriver,
GR-Linux-NIC-Dev, bridge
[-- Attachment #1: Type: text/plain, Size: 8678 bytes --]
On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>
> On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
> > On 24/04/2022 15:09, Alaa Mohamed wrote:
> > > Add extack support to .ndo_fdb_del in netdevice.h and
> > > all related methods.
> > >
> > > Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
> > > ---
> > > changes in V3:
> > > fix errors reported by checkpatch.pl
> > > ---
> > > drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
> > > drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
> > > drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
> > > drivers/net/macvlan.c | 2 +-
> > > drivers/net/vxlan/vxlan_core.c | 2 +-
> > > include/linux/netdevice.h | 2 +-
> > > net/bridge/br_fdb.c | 2 +-
> > > net/bridge/br_private.h | 2 +-
> > > net/core/rtnetlink.c | 4 ++--
> > > 9 files changed, 12 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> > > b/drivers/net/ethernet/intel/ice/ice_main.c
> > > index d768925785ca..7b55d8d94803 100644
> > > --- a/drivers/net/ethernet/intel/ice/ice_main.c
> > > +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> > > @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr
> > > __always_unused *tb[],
> > > static int
> > > ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
> > > struct net_device *dev, const unsigned char *addr,
> > > - __always_unused u16 vid)
> > > + __always_unused u16 vid, struct netlink_ext_ack *extack)
> > > {
> > > int err;
> > > -
> > > +
> > What's changed here?
>
> In the previous version, I removed the blank line after "int err;" and you
> said I shouldn't so I added blank line.
This answer doesn't make sense. You should make a patch against the code
as it is in the kernel, not against your previous proposal.
julia
>
> >
> > > if (ndm->ndm_state & NUD_PERMANENT) {
> > > netdev_err(dev, "FDB only supports static addresses\n");
> > > return -EINVAL;
> > > diff --git a/drivers/net/ethernet/mscc/ocelot_net.c
> > > b/drivers/net/ethernet/mscc/ocelot_net.c
> > > index 247bc105bdd2..e07c64e3159c 100644
> > > --- a/drivers/net/ethernet/mscc/ocelot_net.c
> > > +++ b/drivers/net/ethernet/mscc/ocelot_net.c
> > > @@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg *ndm,
> > > struct nlattr *tb[],
> > >
> > > static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> > > struct net_device *dev,
> > > - const unsigned char *addr, u16 vid)
> > > + const unsigned char *addr, u16 vid, struct
> > > netlink_ext_ack *extack)
> > > {
> > > struct ocelot_port_private *priv = netdev_priv(dev);
> > > struct ocelot_port *ocelot_port = &priv->port;
> > > struct ocelot *ocelot = ocelot_port->ocelot;
> > > int port = priv->chip_port;
> > >
> > > - return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge);
> > > + return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge,
> > > extack);
> > > }
> > >
> > > static int ocelot_port_fdb_dump(struct sk_buff *skb,
> > > diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > index d320567b2cca..51fa23418f6a 100644
> > > --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > @@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device *netdev,
> > > void *p)
> > >
> > > static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> > > struct net_device *netdev,
> > > - const unsigned char *addr, u16 vid)
> > > + const unsigned char *addr, u16 vid, struct
> > > netlink_ext_ack *extack)
> > > {
> > > struct qlcnic_adapter *adapter = netdev_priv(netdev);
> > > int err = -EOPNOTSUPP;
> > > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> > > index 069e8824c264..ffd34d9f7049 100644
> > > --- a/drivers/net/macvlan.c
> > > +++ b/drivers/net/macvlan.c
> > > @@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct
> > > nlattr *tb[],
> > >
> > > static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> > > struct net_device *dev,
> > > - const unsigned char *addr, u16 vid)
> > > + const unsigned char *addr, u16 vid, struct
> > > netlink_ext_ack *extack)
> > > {
> > > struct macvlan_dev *vlan = netdev_priv(dev);
> > > int err = -EINVAL;
> > > diff --git a/drivers/net/vxlan/vxlan_core.c
> > > b/drivers/net/vxlan/vxlan_core.c
> > > index de97ff98d36e..cf2f60037340 100644
> > > --- a/drivers/net/vxlan/vxlan_core.c
> > > +++ b/drivers/net/vxlan/vxlan_core.c
> > > @@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
> > > /* Delete entry (via netlink) */
> > > static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> > > struct net_device *dev,
> > > - const unsigned char *addr, u16 vid)
> > > + const unsigned char *addr, u16 vid, struct
> > > netlink_ext_ack *extack)
> > > {
> > > struct vxlan_dev *vxlan = netdev_priv(dev);
> > > union vxlan_addr ip;
> > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > index 28ea4f8269d4..d0d2a8f33c73 100644
> > > --- a/include/linux/netdevice.h
> > > +++ b/include/linux/netdevice.h
> > > @@ -1509,7 +1509,7 @@ struct net_device_ops {
> > > struct nlattr *tb[],
> > > struct net_device *dev,
> > > const unsigned char *addr,
> > > - u16 vid);
> > > + u16 vid, struct netlink_ext_ack
> > > *extack);
> > > int (*ndo_fdb_dump)(struct sk_buff *skb,
> > > struct netlink_callback *cb,
> > > struct net_device *dev,
> > > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> > > index 6ccda68bd473..5bfce2e9a553 100644
> > > --- a/net/bridge/br_fdb.c
> > > +++ b/net/bridge/br_fdb.c
> > > @@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge *br,
> > > /* Remove neighbor entry with RTM_DELNEIGH */
> > > int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> > > struct net_device *dev,
> > > - const unsigned char *addr, u16 vid)
> > > + const unsigned char *addr, u16 vid, struct netlink_ext_ack
> > > *extack)
> > > {
> > > struct net_bridge_vlan_group *vg;
> > > struct net_bridge_port *p = NULL;
> > > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> > > index 18ccc3d5d296..95348c1c9ce5 100644
> > > --- a/net/bridge/br_private.h
> > > +++ b/net/bridge/br_private.h
> > > @@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct
> > > net_bridge_port *source,
> > > const unsigned char *addr, u16 vid, unsigned long flags);
> > >
> > > int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> > > - struct net_device *dev, const unsigned char *addr, u16 vid);
> > > + struct net_device *dev, const unsigned char *addr, u16 vid,
> > > struct netlink_ext_ack *extack);
> > This is way too long (111 chars) and checkpatch should've complained about
> > it.
> > WARNING: line length of 111 exceeds 100 columns
> > #234: FILE: net/bridge/br_private.h:782:
> > + struct net_device *dev, const unsigned char *addr, u16 vid,
> > struct netlink_ext_ack *extack);
>
> I will fix it.
>
> >
> > > int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device
> > > *dev,
> > > const unsigned char *addr, u16 vid, u16 nlh_flags,
> > > struct netlink_ext_ack *extack);
> > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > > index 4041b3e2e8ec..99b30ae58a47 100644
> > > --- a/net/core/rtnetlink.c
> > > +++ b/net/core/rtnetlink.c
> > > @@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct
> > > nlmsghdr *nlh,
> > > const struct net_device_ops *ops = br_dev->netdev_ops;
> > >
> > > if (ops->ndo_fdb_del)
> > > - err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
> > > + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid,
> > > extack);
> > >
> > > if (err)
> > > goto out;
> > > @@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct
> > > nlmsghdr *nlh,
> > > if (ndm->ndm_flags & NTF_SELF) {
> > > if (dev->netdev_ops->ndo_fdb_del)
> > > err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
> > > - vid);
> > > + vid, extack);
> > > else
> > > err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
> > >
> > > --
> > > 2.36.0
> > >
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
2022-04-24 21:52 ` Nikolay Aleksandrov
@ 2022-04-25 11:47 ` Alaa Mohamed
0 siblings, 0 replies; 18+ messages in thread
From: Alaa Mohamed @ 2022-04-25 11:47 UTC (permalink / raw)
To: Nikolay Aleksandrov, netdev
Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
On ٢٤/٤/٢٠٢٢ ٢٣:٥٢, Nikolay Aleksandrov wrote:
> On 4/25/22 00:09, Alaa Mohamed wrote:
>>
>> On ٢٤/٤/٢٠٢٢ ٢١:٥٥, Nikolay Aleksandrov wrote:
>>> On 24/04/2022 22:49, Alaa Mohamed wrote:
>>>> On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
>>>>> On 24/04/2022 15:09, Alaa Mohamed wrote:
>>>>>> Add extack support to .ndo_fdb_del in netdevice.h and
>>>>>> all related methods.
>>>>>>
>>>>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>>>>> ---
>>>>>> changes in V3:
>>>>>> fix errors reported by checkpatch.pl
>>>>>> ---
>>>>>> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
>>>>>> drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
>>>>>> drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
>>>>>> drivers/net/macvlan.c | 2 +-
>>>>>> drivers/net/vxlan/vxlan_core.c | 2 +-
>>>>>> include/linux/netdevice.h | 2 +-
>>>>>> net/bridge/br_fdb.c | 2 +-
>>>>>> net/bridge/br_private.h | 2 +-
>>>>>> net/core/rtnetlink.c | 4 ++--
>>>>>> 9 files changed, 12 insertions(+), 12 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
>>>>>> b/drivers/net/ethernet/intel/ice/ice_main.c
>>>>>> index d768925785ca..7b55d8d94803 100644
>>>>>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>>>>>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>>>>>> @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct
>>>>>> nlattr __always_unused *tb[],
>>>>>> static int
>>>>>> ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr
>>>>>> *tb[],
>>>>>> struct net_device *dev, const unsigned char *addr,
>>>>>> - __always_unused u16 vid)
>>>>>> + __always_unused u16 vid, struct netlink_ext_ack *extack)
>>>>>> {
>>>>>> int err;
>>>>>> -
>>>>>> +
>>>>> What's changed here?
>>>> In the previous version, I removed the blank line after "int err;"
>>>> and you said I shouldn't so I added blank line.
>>>>
>>> Yeah, my question is are you fixing a dos ending or something else?
>>> The blank line is already there, what's wrong with it?
>> No, I didn't.
>>>
>>> The point is it's not nice to mix style fixes and other changes,
>>> more so
>>> if nothing is mentioned in the commit message.
>> Got it, So, what should I do to fix it?
>
> Don't change that line? I mean I'm even surprised this made it in the
> patch. As I mentioned above, there is already a new line there so I'm
> not sure how you're removing it and adding it again. :)
>
> Cheers,
> Nik
Thanks Nik, I will fix this.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
2022-04-25 6:11 ` Julia Lawall
@ 2022-04-25 11:51 ` Alaa Mohamed
0 siblings, 0 replies; 18+ messages in thread
From: Alaa Mohamed @ 2022-04-25 11:51 UTC (permalink / raw)
To: Julia Lawall
Cc: Nikolay Aleksandrov, netdev, outreachy, roopa, jdenham, sbrivio,
jesse.brandeburg, anthony.l.nguyen, davem, kuba, pabeni,
vladimir.oltean, claudiu.manoil, alexandre.belloni, shshaikh,
manishc, intel-wired-lan, linux-kernel, UNGLinuxDriver,
GR-Linux-NIC-Dev, bridge
On ٢٥/٤/٢٠٢٢ ٠٨:١١, Julia Lawall wrote:
>
> On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>
>> On ٢٤/٤/٢٠٢٢ ٢١:٥٥, Nikolay Aleksandrov wrote:
>>> On 24/04/2022 22:49, Alaa Mohamed wrote:
>>>> On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
>>>>> On 24/04/2022 15:09, Alaa Mohamed wrote:
>>>>>> Add extack support to .ndo_fdb_del in netdevice.h and
>>>>>> all related methods.
>>>>>>
>>>>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>>>>> ---
>>>>>> changes in V3:
>>>>>> fix errors reported by checkpatch.pl
>>>>>> ---
>>>>>> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
>>>>>> drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
>>>>>> drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
>>>>>> drivers/net/macvlan.c | 2 +-
>>>>>> drivers/net/vxlan/vxlan_core.c | 2 +-
>>>>>> include/linux/netdevice.h | 2 +-
>>>>>> net/bridge/br_fdb.c | 2 +-
>>>>>> net/bridge/br_private.h | 2 +-
>>>>>> net/core/rtnetlink.c | 4 ++--
>>>>>> 9 files changed, 12 insertions(+), 12 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
>>>>>> b/drivers/net/ethernet/intel/ice/ice_main.c
>>>>>> index d768925785ca..7b55d8d94803 100644
>>>>>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>>>>>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>>>>>> @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr
>>>>>> __always_unused *tb[],
>>>>>> static int
>>>>>> ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
>>>>>> struct net_device *dev, const unsigned char *addr,
>>>>>> - __always_unused u16 vid)
>>>>>> + __always_unused u16 vid, struct netlink_ext_ack *extack)
>>>>>> {
>>>>>> int err;
>>>>>> -
>>>>>> +
>>>>> What's changed here?
>>>> In the previous version, I removed the blank line after "int err;" and you
>>>> said I shouldn't so I added blank line.
>>>>
>>> Yeah, my question is are you fixing a dos ending or something else?
>>> The blank line is already there, what's wrong with it?
>> No, I didn't.
> OK, so what is the answer to the question about what changed? It looks
> like you remove a blank line and then add it back. But that should not
> show up as a difference when you generate the patch.
>
> When you answer a comment, please put a blank line before and after your
> answer. Otherwise it can be hard to see your answer when it is in the
> middle of a larger patch.
>
>>> The point is it's not nice to mix style fixes and other changes, more so
>>> if nothing is mentioned in the commit message.
>> Got it, So, what should I do to fix it?
> A series? But it is not clear that any change is needed here at all.
>
> julia
I understood but I mean how to create the patch without this but I will
google it , Thanks Julia.
>
>>>>>> if (ndm->ndm_state & NUD_PERMANENT) {
>>>>>> netdev_err(dev, "FDB only supports static addresses\n");
>>>>>> return -EINVAL;
>>>>>> diff --git a/drivers/net/ethernet/mscc/ocelot_net.c
>>>>>> b/drivers/net/ethernet/mscc/ocelot_net.c
>>>>>> index 247bc105bdd2..e07c64e3159c 100644
>>>>>> --- a/drivers/net/ethernet/mscc/ocelot_net.c
>>>>>> +++ b/drivers/net/ethernet/mscc/ocelot_net.c
>>>>>> @@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg
>>>>>> *ndm, struct nlattr *tb[],
>>>>>>
>>>>>> static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr
>>>>>> *tb[],
>>>>>> struct net_device *dev,
>>>>>> - const unsigned char *addr, u16 vid)
>>>>>> + const unsigned char *addr, u16 vid, struct
>>>>>> netlink_ext_ack *extack)
>>>>>> {
>>>>>> struct ocelot_port_private *priv = netdev_priv(dev);
>>>>>> struct ocelot_port *ocelot_port = &priv->port;
>>>>>> struct ocelot *ocelot = ocelot_port->ocelot;
>>>>>> int port = priv->chip_port;
>>>>>>
>>>>>> - return ocelot_fdb_del(ocelot, port, addr, vid,
>>>>>> ocelot_port->bridge);
>>>>>> + return ocelot_fdb_del(ocelot, port, addr, vid,
>>>>>> ocelot_port->bridge, extack);
>>>>>> }
>>>>>>
>>>>>> static int ocelot_port_fdb_dump(struct sk_buff *skb,
>>>>>> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>>>>> b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>>>>> index d320567b2cca..51fa23418f6a 100644
>>>>>> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>>>>> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>>>>> @@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device
>>>>>> *netdev, void *p)
>>>>>>
>>>>>> static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>>>>> struct net_device *netdev,
>>>>>> - const unsigned char *addr, u16 vid)
>>>>>> + const unsigned char *addr, u16 vid, struct
>>>>>> netlink_ext_ack *extack)
>>>>>> {
>>>>>> struct qlcnic_adapter *adapter = netdev_priv(netdev);
>>>>>> int err = -EOPNOTSUPP;
>>>>>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>>>>>> index 069e8824c264..ffd34d9f7049 100644
>>>>>> --- a/drivers/net/macvlan.c
>>>>>> +++ b/drivers/net/macvlan.c
>>>>>> @@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm,
>>>>>> struct nlattr *tb[],
>>>>>>
>>>>>> static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>>>>> struct net_device *dev,
>>>>>> - const unsigned char *addr, u16 vid)
>>>>>> + const unsigned char *addr, u16 vid, struct
>>>>>> netlink_ext_ack *extack)
>>>>>> {
>>>>>> struct macvlan_dev *vlan = netdev_priv(dev);
>>>>>> int err = -EINVAL;
>>>>>> diff --git a/drivers/net/vxlan/vxlan_core.c
>>>>>> b/drivers/net/vxlan/vxlan_core.c
>>>>>> index de97ff98d36e..cf2f60037340 100644
>>>>>> --- a/drivers/net/vxlan/vxlan_core.c
>>>>>> +++ b/drivers/net/vxlan/vxlan_core.c
>>>>>> @@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
>>>>>> /* Delete entry (via netlink) */
>>>>>> static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>>>>> struct net_device *dev,
>>>>>> - const unsigned char *addr, u16 vid)
>>>>>> + const unsigned char *addr, u16 vid, struct
>>>>>> netlink_ext_ack *extack)
>>>>>> {
>>>>>> struct vxlan_dev *vxlan = netdev_priv(dev);
>>>>>> union vxlan_addr ip;
>>>>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>>>>> index 28ea4f8269d4..d0d2a8f33c73 100644
>>>>>> --- a/include/linux/netdevice.h
>>>>>> +++ b/include/linux/netdevice.h
>>>>>> @@ -1509,7 +1509,7 @@ struct net_device_ops {
>>>>>> struct nlattr *tb[],
>>>>>> struct net_device *dev,
>>>>>> const unsigned char *addr,
>>>>>> - u16 vid);
>>>>>> + u16 vid, struct netlink_ext_ack *extack);
>>>>>> int (*ndo_fdb_dump)(struct sk_buff *skb,
>>>>>> struct netlink_callback *cb,
>>>>>> struct net_device *dev,
>>>>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>>>>> index 6ccda68bd473..5bfce2e9a553 100644
>>>>>> --- a/net/bridge/br_fdb.c
>>>>>> +++ b/net/bridge/br_fdb.c
>>>>>> @@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge
>>>>>> *br,
>>>>>> /* Remove neighbor entry with RTM_DELNEIGH */
>>>>>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>>>>> struct net_device *dev,
>>>>>> - const unsigned char *addr, u16 vid)
>>>>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack
>>>>>> *extack)
>>>>>> {
>>>>>> struct net_bridge_vlan_group *vg;
>>>>>> struct net_bridge_port *p = NULL;
>>>>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>>>>> index 18ccc3d5d296..95348c1c9ce5 100644
>>>>>> --- a/net/bridge/br_private.h
>>>>>> +++ b/net/bridge/br_private.h
>>>>>> @@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct
>>>>>> net_bridge_port *source,
>>>>>> const unsigned char *addr, u16 vid, unsigned long
>>>>>> flags);
>>>>>>
>>>>>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>>>>> - struct net_device *dev, const unsigned char *addr, u16
>>>>>> vid);
>>>>>> + struct net_device *dev, const unsigned char *addr, u16 vid,
>>>>>> struct netlink_ext_ack *extack);
>>>>> This is way too long (111 chars) and checkpatch should've complained
>>>>> about it.
>>>>> WARNING: line length of 111 exceeds 100 columns
>>>>> #234: FILE: net/bridge/br_private.h:782:
>>>>> + struct net_device *dev, const unsigned char *addr, u16 vid,
>>>>> struct netlink_ext_ack *extack);
>>>> I will fix it.
>>>>
>>>>>> int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct
>>>>>> net_device *dev,
>>>>>> const unsigned char *addr, u16 vid, u16 nlh_flags,
>>>>>> struct netlink_ext_ack *extack);
>>>>>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>>>>>> index 4041b3e2e8ec..99b30ae58a47 100644
>>>>>> --- a/net/core/rtnetlink.c
>>>>>> +++ b/net/core/rtnetlink.c
>>>>>> @@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb,
>>>>>> struct nlmsghdr *nlh,
>>>>>> const struct net_device_ops *ops = br_dev->netdev_ops;
>>>>>>
>>>>>> if (ops->ndo_fdb_del)
>>>>>> - err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
>>>>>> + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid, extack);
>>>>>>
>>>>>> if (err)
>>>>>> goto out;
>>>>>> @@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb,
>>>>>> struct nlmsghdr *nlh,
>>>>>> if (ndm->ndm_flags & NTF_SELF) {
>>>>>> if (dev->netdev_ops->ndo_fdb_del)
>>>>>> err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
>>>>>> - vid);
>>>>>> + vid, extack);
>>>>>> else
>>>>>> err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
>>>>>>
>>>>>> --
>>>>>> 2.36.0
>>>>>>
> >
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2022-04-25 11:51 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-24 12:09 [PATCH net-next v3 0/2] propagate extack to vxlan_fdb_delete Alaa Mohamed
2022-04-24 12:09 ` [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers Alaa Mohamed
2022-04-24 19:02 ` Nikolay Aleksandrov
2022-04-24 19:49 ` Alaa Mohamed
2022-04-24 19:55 ` Nikolay Aleksandrov
2022-04-24 21:09 ` Alaa Mohamed
2022-04-24 21:52 ` Nikolay Aleksandrov
2022-04-25 11:47 ` Alaa Mohamed
2022-04-25 6:11 ` Julia Lawall
2022-04-25 11:51 ` Alaa Mohamed
2022-04-25 6:12 ` Julia Lawall
2022-04-24 12:09 ` [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete Alaa Mohamed
2022-04-24 16:15 ` Julia Lawall
2022-04-24 18:52 ` Alaa Mohamed
2022-04-24 18:56 ` Julia Lawall
2022-04-24 18:59 ` Alaa Mohamed
2022-04-24 19:03 ` Nikolay Aleksandrov
2022-04-24 19:20 ` Alaa Mohamed
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).