Bug 55565
| Summary: | ipip module bug | ||
|---|---|---|---|
| Product: | [Retired] Red Hat Linux | Reporter: | Yu-Shun Wang <yushunwa> |
| Component: | kernel | Assignee: | Arjan van de Ven <arjanv> |
| Status: | CLOSED UPSTREAM | QA Contact: | Brock Organ <borgan> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | 7.2 | CC: | davem, yushunwa |
| Target Milestone: | --- | ||
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2003-06-10 19:03:38 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
Your patch appears to be in "reversed"; but that's not really a problem.
Dave: just above the patch we do a
memcpy(&nt->parms, parms, sizeof(*parms));so the strcpy to keep
"nt->parms" consistent with parms seems to make sense; I might miss something
subtle though. Does this look correct ?
Yes, but when you use "ip tunnel add" without specifying a name, parms->name is null at the line you indicate. So the newly created name was never assigned to nt->parms.name (or dev->priv.name, they are the same) which was later used to locate the whole data structure when you issued "ip tunnel change". This used to work in 7.0 (2.2.*, not 7.1), so I went back to dig up the old ipip.c, the following line in 7.2, strcpy(dev->name, nt->parms.name); was originally dev->name = nt->parms.name; That's why the patch wasn't necessary in 7.0. A bit off-topic, but it seemed kind of stange to me that iproute2 is not part of the core linux distribution (as in ifconfig, route, etc.) Or maybe I am just not very familiar with how "core" is defined. :-) Also, it would be nice if "ip tunnel add" command would return the name of the newly created tunl interface. But that probably doesn't belong to this bug report. This suggested change is totally wrong. Look at every tunneling driver (ip_gre.c, ipv6/sit.c) they all behave this way. And I am confident that the current behavior is the desired one. Ask Alexey :-) Why? If "ip tunnel add" allows user to give no name (which I totally support), I don't see why it's wrong to return the name of tunnel interface on success. Sure make it a lot easier to script it. (Don't have to scroll through ifconfig to find the new one.) I could be wrong since I only use ipip and have to claim ignorance on sit/gre ones. :-) (The email sent by submit is a bit misleading since it automatically attached the bottom 3 lines of the last comments.) Regarding the patch, if "ip tunnel add" can be used without specifying a name, then there's something wrong with ipip.c code since you can not use "ip tunnel change tunlXX" to modify the parameters of the newly created tunnel. Should we allow users to ask for tunnel without specifying a name? I think so. This save the user trouble to browse through ifconfig every time a new tunnel is needed, and the code (ipip.c) is written to support such behavior, IMHO. |
From Bugzilla Helper: User-Agent: Mozilla/4.78 [en] (X11; U; Linux 2.2.12 i386) Description of problem: The following commands don't work anymore: % ip tunnel add mode ipip .... (without giving a name) (assume this creates tunl5) % ip tunnel change tunl5 .... ! ioctl error ! This happened on ipip.c version 1.42 and 1.46. The attached patch fixed it in 1.46. Please modify the patch as you see fits since I am not very familar with kernel mod. Version-Release number of selected component (if applicable): 1.46 How reproducible: Always Steps to Reproduce: 1. % ip tunnel add mode ipip remote 10.1.1.1 local 10.2.2.2 2. Assume this command creates tunl2. 3. % ip tunnel change tunl2 mode ipip remote 10.3.3.3 local 10.2.2.2 4. You get the following error message: "ioctl: No such file or directory"! Actual Results: ioctl: No such file or directory Expected Results: No error. Additional info: The following patch to net/ipv4/ipip.c fixes it. --- ipip.c.7.2-patched Wed Oct 31 10:04:46 2001 +++ ipip.c.7.2 Wed Oct 31 09:59:59 2001 @@ -256,7 +256,6 @@ if (i==100) goto failed; memcpy(parms->name, dev->name, IFNAMSIZ); - strcpy(nt->parms.name, dev->name); } if (register_netdevice(dev) < 0) goto failed;