Message ID | 0d09ad611bb78b9113491007955f2211f3a06e82.1650800975.git.eng.alaamohamedsoliman.am@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | propagate extack to vxlan_fdb_delete | expand |
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 > > >
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 >> >> >>
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 > > > > > > > > > >
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 >>>> >>>> >>>> > >
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");
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
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;
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(-) -- 2.36.0