From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 597EFA04FD; Wed, 28 Dec 2022 15:12:26 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EE23D40698; Wed, 28 Dec 2022 15:12:25 +0100 (CET) Received: from mail-yw1-f171.google.com (mail-yw1-f171.google.com [209.85.128.171]) by mails.dpdk.org (Postfix) with ESMTP id 0D9D740141 for ; Wed, 28 Dec 2022 15:12:24 +0100 (CET) Received: by mail-yw1-f171.google.com with SMTP id 00721157ae682-476e643d1d5so107002617b3.1 for ; Wed, 28 Dec 2022 06:12:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=fuXd46GSOXFdDPJ6/YEPMET5LfvVqAh+W6ojszMNnkA=; b=GCL5DyH4UwyUsB27XqqUB+p4uIovsWSzMaiG1dyafuAIRulk2w9ECIdyaeUzuMqhgn /GOHrKgOsY577H7rQMHlMiOpj1uju3la/25zu4RjxXcnjrqogL4SJNmrwDchWqDBqenf j5JQJeEBLvUu9xTffl7bi5r61dAa6pfe4DiVIRaXHGaEtt8LPG+Lh2cCm6ry8ftVONbO /gvyp5P8fwVFL9Qo2lFPrigaFs9mISWlr0/UQsOw/p6LyedDEWUjPne8DcOt88/M+I/n 9NgmljcL7tEU70H0n8Mtjl8YvbQTEjduPh1Bh6kB3LgGkrfKCj6HW8QwAlxOMQcPoMtY e8OA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=fuXd46GSOXFdDPJ6/YEPMET5LfvVqAh+W6ojszMNnkA=; b=ZTgWlQ/CtsKT0okQgcN8wMEEijv0fwsanIyviNi0zcgitx0w3i9ikMLdxtxeWjzUgk VS2Kg0PdN2zZrQ1aWNN6278hm2CHKfOztK328wW/S7kytfj9/nErT7dKDXpqD2wpfOyw nD8oIMY4tMEz3BT9u2ipIE4VGKfirwEql/v9OXXmmI67Ut7K9vqcglR5sqGENN8E1MlG jDQrarydaPJaD3TiR4medUryMuZKpZi/HDx5QtlP6TWAnzQLQOnqXjHOKiuYbMVDvEaO 1og8tdZLqKQ56yxPiYt/0zz59oHCwVQOk6efSCFdOX7IUEvJbVlBuIPyJVGVZUHR/0VI 3qiQ== X-Gm-Message-State: AFqh2kqB3XcH2QHZhc5h2bFOevg3oiUNlNSlSRKKU3bCfo0i8GEUSZbA +Pod9HOLLZsg/xHbDqRpPBpAuO0xIItVXipNZYc= X-Google-Smtp-Source: AMrXdXvmkVu360dz/ToZjjqHOE5wAmXw7TC0V2wkAIWB5jNgiAh9htEV2ncNht1YPRcvrndDq2R2vB++h0L5ZQgcmpc= X-Received: by 2002:a81:92cc:0:b0:440:c35b:a557 with SMTP id j195-20020a8192cc000000b00440c35ba557mr2595950ywg.180.1672236743191; Wed, 28 Dec 2022 06:12:23 -0800 (PST) MIME-Version: 1.0 References: <20221227084311.54d1fe96@hermes.local> In-Reply-To: <20221227084311.54d1fe96@hermes.local> From: Ben Magistro Date: Wed, 28 Dec 2022 09:12:12 -0500 Message-ID: Subject: Re: dumpcap, interfaces, and promiscuous mode To: Stephen Hemminger Cc: dev@dpdk.org, "Kaur, Arshdeep" , ben.magistro@trinitycyber.com Content-Type: multipart/alternative; boundary="0000000000000ef22605f0e3f575" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --0000000000000ef22605f0e3f575 Content-Type: text/plain; charset="UTF-8" I would like to respectfully ask that you please re-read my initial email. Unfortunately the interface selection issue I describe is not resolved in 22.11 (currently running). It is also easily observed by reviewing the current code (once one knows to look for it). I'll be the first to admit I did not catch it when looking at the patch. To try and summarize it again here, it is trying to store an array of interfaces (multiple -i options) into a single char variable. The only interface that is persisted to selection is the last one specified because it overwrites the previously saved interface(s). If the intent is to only support capturing on a single interface then the asterisk option should be removed, it should become an error to specify the interface multiple times, and the associated documentation should be updated to reflect this. However, I do not believe the intent is to limit capture to a single interface and would not support such a change. I can understand modeling dumpcap off the wireshark version but DPDK brings its own unique capabilities and I believe those need to be accounted for too. Looking at the man page of wireshark's dumpcap, it indicates that the "-p" option can be specified multiple times and it needs to be relative to the interface [1], if it is not it may be ignored. Implementing this change seems like it would bring DPDK dumpcap closer to the behavior of wireshark's dumpcap when specifying the interface's promiscuous mode. The above promiscuous mode change does not address the potential issue of stopping a capture and it changing an interface's promiscuous mode for the main application that continues to run. The only path forward here that I can see is storing the initial promiscuous mode state on each interface that a capture is occurring on and checking that to determine what should be done when stopping dumpcap. This leaves the last question of being able to just inherit the main process's promiscuous state so that a user doesn't necessarily need to know which interfaces are in which state when starting dumpcap to troubleshoot something. Tying in to the previous about storing the state, it would potentially avoid a user starting all interfaces in promiscuous mode which might result in different traffic in the capture than the application normally sees. Finally after looking at the wireshark dumpcap man page, I believe there is a new issue here as well. The man page states that if a capture will occur on multiple interfaces, the file will be saved in pcapng format. Quickly at the code it does not look like this is the current behavior of DPDK dumpcap. As mentioned before I am happy to try and work on some of these changes, but would like to have something of a plan before starting that work. Cheers, Ben Magistro [1] https://www.wireshark.org/docs/man-pages/dumpcap.html On Tue, Dec 27, 2022 at 11:43 AM Stephen Hemminger < stephen@networkplumber.org> wrote: > On Tue, 27 Dec 2022 09:26:14 -0500 > Ben Magistro wrote: > > > I'd like to start out by saying what I believed to be a simple change > > (a8dde09 ("app/dumpcap: allow help/version without primary process")) > seems > > to have had more ripples than I anticipated. I'd like to just get a > > conversation going here before developing/submitting a patch. I think > this > > will likely need to be at least two patches just to keep scope in check. > > > > With regard to interface selection, the most recent patch (7f3623a > > ("app/dumpcap: fix select interface")) breaks capturing on > > multiple interfaces. You can still specify the `-i` option as many times > > as you would like but only the last entry is used in the capture > selection > > as each entry just overwrites the previous entry. I believe this needs > to > > be flipped to an array or similar structure that can have entries > > appended to it as the arguments are processed. Selecting all interfaces > > with the asterisk seems to be unaffected but could also result in > capturing > > traffic on unnecessary interfaces. > > > > With regard to promiscuous mode, there are two areas of concern here. > The > > first is this flag is global and not per interface. I can envision a > > scenario where you might want to capture on one interface in promiscuous > > mode and on a second not in promiscuous mode. My first thought is to > make > > this option follow the interface parameter then when parsing arguments so > > that it can be associated with a specific interface. The second is that > if > > I run a capture in promiscuous mode and then stop the capture, it will > also > > disable promiscuous mode. Generally I think I would agree with this > > behavior as it follows how a typical call to tcpdump should behave. The > > concern here though is that that the main process may have been > > started/running with promiscuous mode and stopping a capture would change > > that mode for the main process. My first thought here is to query the > > initial state and check that when quitting to know if it should disable > > promiscuous mode or not. This brings me to another aspect here and I > don't > > think changing the behaviour of the `-p` flag is appropriate, but can see > > maybe adding an option to inherit the main process's promiscuous state(s) > > when starting. > > > > Happy to try and work on some of these changes but want to talk through > the > > issues first so we can try to address all of them. > > > > Cheers, > > > > Ben Magistro > > I believe all this got fixed by the 22.11 version. > Since dumpcap has interface parameter, it should only enable the ones it > is using. > > The user interface for the DPDK version of dumpcap is modeled after > the wireshark version. In general would not like to invent lots of new > DPDK specific options here. > --0000000000000ef22605f0e3f575 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
I would like to respectfully ask that you please re-r= ead my initial=C2=A0email.

Unfortunately the=C2=A0= interface selection issue=C2=A0I describe is not resolved in 22.11 (current= ly=C2=A0running).=C2=A0 It=C2=A0is also easily observed by=C2=A0reviewing t= he current code (once one knows to look for it).=C2=A0 I'll be the firs= t to admit I did not catch=C2=A0it when looking at=C2=A0the patch.=C2=A0 To= try and summarize it again here, it is trying to store an array of interfa= ces (multiple -i options) into a single char variable.=C2=A0 The only inter= face that is persisted=C2=A0to selection is the last one specified because = it overwrites the previously=C2=A0saved interface(s).=C2=A0 If the intent i= s to only support capturing on a single interface then the asterisk option = should be removed, it should become an error to specify the interface multi= ple=C2=A0times, and the associated documentation should be updated=C2=A0to = reflect this.=C2=A0 However, I do not believe the intent is to limit captur= e to a single interface and would not support such a change.

=
I can understand modeling dumpcap off the wireshark version but = DPDK brings its own unique capabilities and I believe those=C2=A0need to be= accounted for too.

Looking at the man page of wir= eshark's dumpcap, it indicates that the "-p" option can be sp= ecified multiple times and it needs to be relative to the interface [1], if= it is not it may be ignored.=C2=A0 Implementing this change seems like it = would bring DPDK dumpcap closer to the behavior of wireshark's dumpcap = when specifying the interface's promiscuous mode.

<= div>The above promiscuous mode change does not address the potential issue = of stopping a capture and it changing an interface's promiscuous mode f= or the main application that=C2=A0continues to run.=C2=A0 The only path for= ward here that I can see is storing the initial promiscuous mode state on e= ach interface that a capture is occurring on and checking that to determine= what should be done when stopping dumpcap.

This l= eaves the last question of being able to just inherit the main process'= s promiscuous=C2=A0state so that a user doesn't necessarily need to kno= w which interfaces are in which state when starting dumpcap to troubleshoot= something.=C2=A0 Tying in to the previous about storing the state, it woul= d potentially avoid a user starting all interfaces in promiscuous mode whic= h might result in different traffic in the capture than the application nor= mally sees.

Finally after looking at the wireshark= dumpcap man page, I believe there is a new issue here as well.=C2=A0 The m= an page states that if a capture will occur on multiple interfaces, the fil= e will be saved in pcapng format.=C2=A0 Quickly at the code it does not loo= k like this is the current behavior of DPDK dumpcap.

As mentioned before I am happy to try and work on some of these changes,= but would like to have something of a plan before starting that work.

Cheers,

Ben Magistro


On Tue, Dec 27, 2022 at 11:43 AM Stephen Hemminger <stephen@networkplumber.org> wrote= :
On Tue, 27 Dec= 2022 09:26:14 -0500
Ben Magistro <ko= ncept1@gmail.com> wrote:

> I'd like to start out by saying what I believed to be a simple cha= nge
> (a8dde09 ("app/dumpcap: allow help/version without primary proces= s")) seems
> to have had more ripples than I anticipated.=C2=A0 I'd like to jus= t get a
> conversation going here before developing/submitting a patch.=C2=A0 I = think this
> will likely need to be at least two patches just to keep scope in chec= k.
>
> With regard to interface selection, the most recent patch (7f3623a
> ("app/dumpcap: fix select interface")) breaks capturing on > multiple interfaces.=C2=A0 You can still specify the `-i` option as ma= ny times
> as you would like but only the last entry is used in the capture selec= tion
> as each entry just overwrites the previous entry.=C2=A0 I believe this= needs to
> be flipped to an array or similar structure that can have entries
> appended to it as the arguments are processed.=C2=A0 Selecting all int= erfaces
> with the asterisk seems to be unaffected but could also result in capt= uring
> traffic on unnecessary interfaces.
>
> With regard to promiscuous mode, there are two areas of concern here.= =C2=A0 The
> first is this flag is global and not per interface.=C2=A0 I can envisi= on a
> scenario where you might want to capture on one interface in promiscuo= us
> mode and on a second not in promiscuous mode.=C2=A0 My first thought i= s to make
> this option follow the interface parameter then when parsing arguments= so
> that it can be associated with a specific interface.=C2=A0 The second = is that if
> I run a capture in promiscuous mode and then stop the capture, it will= also
> disable promiscuous mode.=C2=A0 Generally I think I would agree with t= his
> behavior as it follows how a typical call to tcpdump should behave.=C2= =A0 The
> concern here though is that that the main process may have been
> started/running with promiscuous mode and stopping a capture would cha= nge
> that mode for the main process.=C2=A0 My first thought here is to quer= y the
> initial state and check that when quitting to know if it should disabl= e
> promiscuous mode or not.=C2=A0 This brings me to another aspect here a= nd I don't
> think changing the behaviour of the `-p` flag is appropriate, but can = see
> maybe adding an option to inherit the main process's promiscuous s= tate(s)
> when starting.
>
> Happy to try and work on some of these changes but want to talk throug= h the
> issues first so we can try to address all of them.
>
> Cheers,
>
> Ben Magistro

I believe all this got fixed by the 22.11 version.
Since dumpcap has interface parameter, it should only enable the ones it is using.

The user interface for the DPDK version of dumpcap is modeled after
the wireshark version. In general would not like to invent lots of new
DPDK specific options here.
--0000000000000ef22605f0e3f575--