Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 24 May 2018 13:11:22 +0300
From:      Konstantin Belousov <kib@freebsd.org>
To:        Xin LI <delphij@gmail.com>
Cc:        Gleb Smirnoff <glebius@freebsd.org>, slavash@freebsd.org, hselasky@freebsd.org, "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, svn-src-vendor@freebsd.org
Subject:   Re: svn commit: r333668 - vendor/tcpdump/dist
Message-ID:  <20180524101122.GR88128@kib.kiev.ua>
In-Reply-To: <CAGMYy3t0%2BxUAKHh985J5zuC-8K35ueDoKm5ebp9O__y6n-xRAg@mail.gmail.com>
References:  <201805160843.w4G8h8oW050800@repo.freebsd.org> <20180523212513.GR71675@FreeBSD.org> <CAGMYy3t0%2BxUAKHh985J5zuC-8K35ueDoKm5ebp9O__y6n-xRAg@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, May 23, 2018 at 02:39:06PM -0700, Xin LI wrote:
> On Wed, May 23, 2018 at 2:25 PM Gleb Smirnoff <glebius@freebsd.org> wrote:
> 
> >    Hi,
> 
> > do we commit cherry-picks into vendor/ subversion space? I believe we
> don't.
> 
> Yes we do.  Actually doing it _properly_ would make future imports easier.
> 
> > Cherry-picks should go directly to head. vendor/ should get only tagged
> > full tree updates.
> 
> No, that would make future merges harder.  Let's say you want to apply a
> patchset P, if it's already accepted by upstream, you would do:
> 
>   - Commit it against vendor/.../dist
>   - "svn merge -c <commit> ^..../dist" or merge -r<earlier commit>:<commit>
> 
> If the patchset was not accepted at the time, but later accepted as-is, you
> would compensate the local change with:
> 
>   - Commit it against vendor/.../dist
>   - "svn merge -c ... --record-only ..."
> 
> Lastly, if the patchset was adopted by upstream with changes, you would do
> a similar change for the first one, but tweak the local copy to reduce diff
> against the cherry-picked dist/ version.
> 
> This way, svn would know that the change is already done in dist/ and
> calculate diff's more accurately for future merges.
Yes, I mostly agree with the description.

Main point is that when the vendor change put into vendor/, next vendor
import, typically done by the untarring the archive of the next revision,
silently override the imported change and everything is fine.  Even such
not too intelligent merge code as in subversion, typically silently
consumes already merged chunks by three-way merge algorithm.

> 
> Note that Slava didn't commit the mergeinfo update in head/, which should
> be done to take advantage of this maneuver.  Without it, svn would have
> problem figuring the right delta.
I was puzzled by this statement, since Slava did not managed to handle
the move of the vendor commit into the contrib/ area yet at all.   Then I
realized that this looks like it was forgotten.  No, it is just slow
and might be finished by somebody else.

> 
> 
> > On Wed, May 16, 2018 at 08:43:08AM +0000, Slava Shwartsman wrote:
> > S> Author: slavash
> > S> Date: Wed May 16 08:43:08 2018
> > S> New Revision: 333668
> > S> URL: https://svnweb.freebsd.org/changeset/base/333668
> > S>
> > S> Log:
> > S>   Vendor import two upstream commits:
> > S>   c1bb8784abd3ca978e376b0d10e324db0491237b
> > S>   9c4af7213cc2543a1f5586d8f2c19f86aa0cbe72
> > S>
> > S>   When using tcpdump -I -i wlanN and wlanN is not a monitor mode VAP,
> > S>   tcpdump will print an error message saying rfmon is not supported.
> > S>
> > S>   Give a concise explanation as to how one might solve this problem by
> > S>   creating a monitor mode VAP.
> > S>
> > S>   Approved by:    hselasky (mentor), kib (mentor)
> > S>   Sponsored by:   Mellanox Technologies
> > S>
> > S> Modified:
> > S>   vendor/tcpdump/dist/tcpdump.c
> > S>
> > S> Modified: vendor/tcpdump/dist/tcpdump.c
> > S>
> ==============================================================================
> > S> --- vendor/tcpdump/dist/tcpdump.c    Wed May 16 06:52:08 2018
>   (r333667)
> > S> +++ vendor/tcpdump/dist/tcpdump.c    Wed May 16 08:43:08 2018
>   (r333668)
> > S> @@ -108,6 +108,10 @@ The Regents of the University of California.  All
> righ
> > S>  #endif /* HAVE_CAP_NG_H */
> > S>  #endif /* HAVE_LIBCAP_NG */
> > S>
> > S> +#ifdef __FreeBSD__
> > S> +#include <sys/sysctl.h>
> > S> +#endif /* __FreeBSD__ */
> > S> +
> > S>  #include "netdissect.h"
> > S>  #include "interface.h"
> > S>  #include "addrtoname.h"
> > S> @@ -1044,6 +1048,30 @@ open_interface(const char *device,
> netdissect_options
> > S>              } else if (status == PCAP_ERROR_PERM_DENIED && *cp !=
> '\0')
> > S>                      error("%s: %s\n(%s)", device,
> > S>                          pcap_statustostr(status), cp);
> > S> +#ifdef __FreeBSD__
> > S> +            else if (status == PCAP_ERROR_RFMON_NOTSUP &&
> > S> +                strncmp(device, "wlan", 4) == 0) {
> > S> +                    char parent[8], newdev[8];
> > S> +                    char sysctl[32];
> > S> +                    size_t s = sizeof(parent);
> > S> +
> > S> +                    snprintf(sysctl, sizeof(sysctl),
> > S> +                        "net.wlan.%d.%%parent", atoi(device + 4));
> > S> +                    sysctlbyname(sysctl, parent, &s, NULL, 0);
> > S> +                    strlcpy(newdev, device, sizeof(newdev));
> > S> +                    /* Suggest a new wlan device. */
> > S> +                    /* FIXME: incrementing the index this way is not
> going to work well
> > S> +                     * when the index is 9 or greater but the only
> consequence in this
> > S> +                     * specific case would be an error message that
> looks a bit odd.
> > S> +                     */
> > S> +                    newdev[strlen(newdev)-1]++;
> > S> +                    error("%s is not a monitor mode VAP\n"
> > S> +                        "To create a new monitor mode VAP use:\n"
> > S> +                        "  ifconfig %s create wlandev %s wlanmode
> monitor\n"
> > S> +                        "and use %s as the tcpdump interface",
> > S> +                        device, newdev, parent, newdev);
> > S> +            }
> > S> +#endif
> > S>              else
> > S>                      error("%s: %s", device,
> > S>                          pcap_statustostr(status));
> > S>
> 
> > --
> > Gleb Smirnoff



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180524101122.GR88128>