| Message ID | 20120425103858.04d149d8@nehalam.linuxnetplumber.net |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Stephen Hemminger <shemminger@vyatta.com> Date: Wed, 25 Apr 2012 10:38:58 -0700 > Use the standard convention to define the number of elements > in unix diag attribute. This fixes future problems like the fact > the last element (MEMINFO) is not parsed by current iproute2 ss command. > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> You can't change this, it's already out in the wild, and for 2 releases. Sorry. Just accept the 'ss' patch that was posted and you objected to, I'm not breaking userspace just so that you don't have to apply a userland patch you perceive as an unacceptable quirk. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 25 Apr 2012 14:16:58 -0400 (EDT) David Miller <davem@davemloft.net> wrote: > From: Stephen Hemminger <shemminger@vyatta.com> > Date: Wed, 25 Apr 2012 10:38:58 -0700 > > > Use the standard convention to define the number of elements > > in unix diag attribute. This fixes future problems like the fact > > the last element (MEMINFO) is not parsed by current iproute2 ss command. > > > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> > > You can't change this, it's already out in the wild, and > for 2 releases. > > Sorry. > > Just accept the 'ss' patch that was posted and you objected to, > I'm not breaking userspace just so that you don't have to apply > a userland patch you perceive as an unacceptable quirk. It only changes a broken enum value used only by iproute2. Do you expect the one utility to use it to have a workaround for a broken initial version. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Stephen Hemminger <shemminger@vyatta.com> Date: Wed, 25 Apr 2012 11:56:09 -0700 > Do you expect the one utility to use it to have a workaround for > a broken initial version. I'm not taking that risk. They thought the same exact thing when they did the autofs struct size compat fix, and it turned out to break things. Stephen I have an awesome suggestion for you if you want to avoid this in the future, review iproute2 patches more aggressively so you can catch things like this earlier. Like, when we can actually still safely change things. Because currently you let patches rot in patchwork. There's an iproute2 patch in there assigned to you which is 3 months old, that simply isn't how this is supposed to work. I hate to keep beating a dead horse, but you don't stay on top of patchwork like you should. The object is not to let patches just rot in "Under Review" state for months. Either you apply them as soon as possible, or you mark them appropriately as "Changes Requested" or "Deferred" so that the submitter makes appropriate fixes you've asked for, or resubmits when it's more appropriate for the change to go in. "Under Review" doesn't mean, "I'm waiting for a kernel release with the feature". But that's how you use it. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 25 Apr 2012 15:07:16 -0400 (EDT) David Miller <davem@davemloft.net> wrote: > From: Stephen Hemminger <shemminger@vyatta.com> > Date: Wed, 25 Apr 2012 11:56:09 -0700 > > > Do you expect the one utility to use it to have a workaround for > > a broken initial version. > > I'm not taking that risk. > > They thought the same exact thing when they did the autofs struct size > compat fix, and it turned out to break things. > > Stephen I have an awesome suggestion for you if you want to avoid this > in the future, review iproute2 patches more aggressively so you can > catch things like this earlier. Like, when we can actually still > safely change things. Sorry it was more of cross project issue in this case. The original kernel patch had the problem and was lost in the fog of the other issues like the unix diag implementation not building. A community works best if multiple people look at the code. Don't think I would have spotted it unless I compared it to other places. > Because currently you let patches rot in patchwork. There's an > iproute2 patch in there assigned to you which is 3 months old, that > simply isn't how this is supposed to work. > > I hate to keep beating a dead horse, but you don't stay on top of > patchwork like you should. The object is not to let patches just > rot in "Under Review" state for months. I keep patches that are for -next in that state. > Either you apply them as soon as possible, or you mark them > appropriately as "Changes Requested" or "Deferred" so that the > submitter makes appropriate fixes you've asked for, or resubmits when > it's more appropriate for the change to go in. > > "Under Review" doesn't mean, "I'm waiting for a kernel release with > the feature". But that's how you use it. Ok. What is the suggested tag for that. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Stephen Hemminger <shemminger@vyatta.com> Date: Wed, 25 Apr 2012 12:47:43 -0700 > On Wed, 25 Apr 2012 15:07:16 -0400 (EDT) > David Miller <davem@davemloft.net> wrote: > >> Either you apply them as soon as possible, or you mark them >> appropriately as "Changes Requested" or "Deferred" so that the >> submitter makes appropriate fixes you've asked for, or resubmits when >> it's more appropriate for the change to go in. >> >> "Under Review" doesn't mean, "I'm waiting for a kernel release with >> the feature". But that's how you use it. > > Ok. What is the suggested tag for that. As I said above, "Deferred", with a note sent to the submitter to resubmit the patch at the appropriate time. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 25 Apr 2012 15:57:17 -0400 (EDT) David Miller <davem@davemloft.net> wrote: > From: Stephen Hemminger <shemminger@vyatta.com> > Date: Wed, 25 Apr 2012 12:47:43 -0700 > > > On Wed, 25 Apr 2012 15:07:16 -0400 (EDT) > > David Miller <davem@davemloft.net> wrote: > > > >> Either you apply them as soon as possible, or you mark them > >> appropriately as "Changes Requested" or "Deferred" so that the > >> submitter makes appropriate fixes you've asked for, or resubmits when > >> it's more appropriate for the change to go in. > >> > >> "Under Review" doesn't mean, "I'm waiting for a kernel release with > >> the feature". But that's how you use it. > > > > Ok. What is the suggested tag for that. > > As I said above, "Deferred", with a note sent to the submitter to resubmit > the patch at the appropriate time. That doesn't work, not asking original submitter to resubmit. I tried a branch, but git doesn't like branches coming and going on remote repos. Probably better to just have an ephemeral iproute2-next repo on kernel.org. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Stephen Hemminger <shemminger@vyatta.com> Date: Wed, 25 Apr 2012 13:09:06 -0700 > That doesn't work, not asking original submitter to resubmit. It works every single time for me. If they don't resubmit, they don't really care about their work being included, and therefore neither should you. End of story. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/include/linux/unix_diag.h 2012-02-13 09:23:58.830508614 -0800 +++ b/include/linux/unix_diag.h 2012-04-25 09:36:20.752056963 -0700 @@ -37,9 +37,9 @@ enum { UNIX_DIAG_ICONS, UNIX_DIAG_RQLEN, UNIX_DIAG_MEMINFO, - - UNIX_DIAG_MAX, + __UNIX_DIAG_MAX }; +#define UNIX_DIAG_MAX (__UNIX_DIAG_MAX - 1) struct unix_diag_vfs { __u32 udiag_vfs_ino;
Use the standard convention to define the number of elements in unix diag attribute. This fixes future problems like the fact the last element (MEMINFO) is not parsed by current iproute2 ss command. Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html