[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e8ca868f-bbd6-c638-310c-d9c36aedb5d3@gmail.com>
Date: Sun, 24 Apr 2022 20:59:44 +0200
From: Alaa Mohamed <eng.alaamohamedsoliman.am@...il.com>
To: Julia Lawall <julia.lawall@...ia.fr>
Cc: netdev@...r.kernel.org, outreachy@...ts.linux.dev,
roopa@...dia.com, jdenham@...hat.com, sbrivio@...hat.com,
jesse.brandeburg@...el.com, anthony.l.nguyen@...el.com,
davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
vladimir.oltean@....com, claudiu.manoil@....com,
alexandre.belloni@...tlin.com, shshaikh@...vell.com,
manishc@...vell.com, razor@...ckwall.org,
intel-wired-lan@...ts.osuosl.org, linux-kernel@...r.kernel.org,
UNGLinuxDriver@...rochip.com, GR-Linux-NIC-Dev@...vell.com,
bridge@...ts.linux-foundation.org
Subject: Re: [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack
support to vxlan_fdb_delete
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@...il.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
>>>>
>>>>
>>>>
> >
Powered by blists - more mailing lists