Re: [Bridge] [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete

[Alaa Mohamed]

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...@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

>