[snip]

@@ -2933,6 +2945,8 @@ static int vxlan_config_validate(struct net *src_net, 
struct vxlan_config *conf,
                  */
                 if ((conf->flags & ~VXLAN_F_ALLOWED_GPE) ||
                     !(conf->flags & VXLAN_F_COLLECT_METADATA)) {
+                       NL_SET_ERR_MSG(extack,
+                                      "VXLAN GPE does not support this combination 
of attributes");
                         return -EINVAL;
                 }


"collect metadata not supported with vxlan gpe"

Actually, VXLAN GPE is only supported together with external keyword. From ip-link(8)...

      gpe - enables the Generic Protocol extension (VXLAN-GPE). Currently, this
           is only supported together with the external keyword.

That's completely wrong message. Not saying that the capitalization is
wrong, too. Girish's wording precisely explains what went wrong.

Furthermore, the condition not only checks for collect metadata but also it checks to see if any attribute specified is outside of what is allowed by VXLAN_F_ALLOWED_GPE.


                 lowerdev = __dev_get_by_index(src_net, conf->remote_ifindex);
-               if (!lowerdev)
+               if (!lowerdev) {
+                       NL_SET_ERR_MSG(extack,
+                                      "Specified interface for tunnel endpoint 
communications not found");
                         return -ENODEV;

"invalid vxlan remote link interface, device not found"

Finally one that looks better :-) Modulo the missing capitalization,
though.

It is not the remote link interface. It is the local interface itself that needs to be specified.


-               if (vxlan_addr_multicast(&conf->remote_ip))
+               if (vxlan_addr_multicast(&conf->remote_ip)) {
+                       NL_SET_ERR_MSG(extack,
+                                      "Interface need to be specified for multicast 
destination");

"vxlan remote link interface required for multicast remote destination"

I like this one better, too.

  #if IS_ENABLED(CONFIG_IPV6)
-               if (conf->flags & VXLAN_F_IPV6_LINKLOCAL)
+               if (conf->flags & VXLAN_F_IPV6_LINKLOCAL) {
+                       NL_SET_ERR_MSG(extack,
+                                      "Interface need to be specified for 
link-local local/remote addresses");
                         return -EINVAL;

"vxlan link interface required for link-local local/remote addresses"

Okay but to be consistent (and more clear), it should be "remote link
interface".

There is no remote link while configuring VXLAN. It is the local interface through which VXLAN packets will be send out.

Thanks all for the review,
~Girish

Reply via email to