* Re: [PATCH 2/3] wireless: iwlwifi: function definition cosmetic fix
From: Luciano Coelho @ 2017-10-04 16:13 UTC (permalink / raw)
To: Christoph Böhmwalder, johannes.berg, emmanuel.grumbach,
kvalo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20171004155700.18048-3-christoph@boehmwalder.at>
On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Separate the function from the previous definition with a newline and
> put the `static` keyword on the same line, as it just looks nicer.
>
> Signed-off-by: Christoph Böhmwalder <christoph@boehmwalder.at>
> ---
> drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> index 0eb815ae97e8..249ee1c7b02f 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> @@ -325,8 +325,8 @@ static u16 channel_id_to_txp(struct iwl_phy_db *phy_db, u16 ch_id)
> }
> return 0xff;
> }
> -static
> -int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
> +
> +static int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
> u32 type, u8 **data, u16 *size, u16 ch_id)
> {
> struct iwl_phy_db_entry *entry;
Sorry, but this now looks much uglier because the second line is not
even aligned to the parenthesis.
NACK.
--
Luca.
^ permalink raw reply
* Re: [PATCH 3/3] wireless: iwlwifi: wrap macro into braces
From: Joe Perches @ 2017-10-04 16:14 UTC (permalink / raw)
To: Christoph Böhmwalder, johannes.berg, emmanuel.grumbach,
luciano.coelho, kvalo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20171004155700.18048-4-christoph@boehmwalder.at>
On Wed, 2017-10-04 at 17:57 +0200, Christoph Böhmwalder wrote:
> Macros should always be wrapped in braces, so fix this instance.
>
> Signed-off-by: Christoph Böhmwalder <christoph@boehmwalder.at>
> ---
> drivers/net/wireless/intel/iwlwifi/iwl-io.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-io.c b/drivers/net/wireless/intel/iwlwifi/iwl-io.c
> index efb1998dcabd..0211963b3e71 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-io.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-io.c
> @@ -252,7 +252,7 @@ IWL_EXPORT_SYMBOL(iwl_force_nmi);
>
> static const char *get_rfh_string(int cmd)
> {
> -#define IWL_CMD(x) case x: return #x
> +#define IWL_CMD(x) { case x: return #x; }
I think this unnecessary. Maybe:
http://lists.infradead.org/pipermail/ath10k/2017-February/009335.html
> #define IWL_CMD_MQ(arg, reg, q) { if (arg == reg(q)) return #reg; }
But this should use do { ... } while (0)
^ permalink raw reply
* Re: [PATCH 1/3] wireless: iwlwifi: use bool instead of int
From: Luciano Coelho @ 2017-10-04 16:15 UTC (permalink / raw)
To: Christoph Böhmwalder, johannes.berg, emmanuel.grumbach,
kvalo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20171004155700.18048-2-christoph@boehmwalder.at>
On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Change a usage of int in a boolean context to use the bool type
> instead, as it
> makes the intent of the function clearer and helps clarify its
> semantics.
>
> Also eliminate the if/else and just return the boolean result
> directly,
> making the code more readable.
>
> Signed-off-by: Christoph Böhmwalder <christoph@boehmwalder.at>
> ---
> drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> index b7cd813ba70f..0eb815ae97e8 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> @@ -267,14 +267,12 @@ int iwl_phy_db_set_section(struct iwl_phy_db
> *phy_db,
> }
> IWL_EXPORT_SYMBOL(iwl_phy_db_set_section);
>
> -static int is_valid_channel(u16 ch_id)
> +static bool is_valid_channel(u16 ch_id)
> {
> - if (ch_id <= 14 ||
> - (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> - (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> - (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1))
> - return 1;
> - return 0;
> + return (ch_id <= 14 ||
> + (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> + (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> + (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1));
> }
>
> static u8 ch_id_to_ch_index(u16 ch_id)
This actually makes some sense, and I would probably apply it if it
were part of a patchset that actually does something useful.
--
Luca.
^ permalink raw reply
* Re: [PATCH net-next 6/7] net: bridge: Pass extack to down to netdev_master_upper_dev_link
From: David Ahern @ 2017-10-04 16:16 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, j.vosburgh, vfalico, andy, jiri, idosch, davem, bridge
In-Reply-To: <20171004081357.37bfd941@xeon-e3>
On 10/4/17 8:13 AM, Stephen Hemminger wrote:
> On Tue, 3 Oct 2017 21:58:53 -0700
> David Ahern <dsahern@gmail.com> wrote:
>
>> Pass extack arg to br_add_if. Add messages for a couple of failures
>> and pass arg to netdev_master_upper_dev_link.
>>
>> Signed-off-by: David Ahern <dsahern@gmail.com>
>
> This looks good. You might want to pass the netlink_ext_ack down as
> an immutable pointer (const).
Can't mark extack as const. The NL_SET_ERR_MSG sets _msg to the passed
in string.
>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
>
^ permalink raw reply
* Re: [PATCH] net: phy: DP83822 initial driver submission
From: Florian Fainelli @ 2017-10-04 16:18 UTC (permalink / raw)
To: Dan Murphy, andrew; +Cc: netdev
In-Reply-To: <86fdd4ac-428c-16a4-2b07-fe67765c6f13@ti.com>
On 10/04/2017 08:41 AM, Dan Murphy wrote:
> Florian
>
> On 10/03/2017 01:31 PM, Florian Fainelli wrote:
>> On 10/03/2017 11:03 AM, Dan Murphy wrote:
>>> Florian
>>>
>>> Thanks for the review
>>>
>>> On 10/03/2017 12:15 PM, Florian Fainelli wrote:
>>>>> + } else {
>>>>> + value &= ~DP83822_WOL_SECURE_ON;
>>>>> + }
>>>>> +
>>>>> + value |= (DP83822_WOL_EN | DP83822_WOL_CLR_INDICATION |
>>>>> + DP83822_WOL_CLR_INDICATION);
>>>>
>>>> The extra parenthesis should not be required here.
>>>
>>> I did not code that in. I had to add it after Checkpatch cribbed about it.
>>> Let me know if you want me to remove it.
>>
>> Let's keep those, that does not change much.
>>
>>>
>>>>
>>>>> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
>>>>> + value);
>>>>> + } else {
>>>>> + value =
>>>>> + phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
>>>>> + value &= (~DP83822_WOL_EN);
>>>>
>>>> Same here, parenthesis should not be needed.
>>>
>>> There are three lines of code in the else. This code all needs to be excuted in the else case.
>>> I might reformat it to read better. Lindent messed that one up.
>>
>> sorry, I meant to write that you don't need the parenthesis around
>> DP83822_WOL_EN since that is just a single bit here.
>>
>> [snip]
>>
>>>>> +
>>>>> + mutex_unlock(&phydev->lock);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static int dp83822_resume(struct phy_device *phydev)
>>>>> +{
>>>>> + int value;
>>>>> +
>>>>> + mutex_lock(&phydev->lock);
>>>>> +
>>>>> + value = phy_read(phydev, MII_BMCR);
>>>>> + phy_write(phydev, MII_BMCR, value & ~BMCR_PDOWN);
>>>>
>>>> And genphy_resume() here as well?
>>>
>>> genphy_resume does not have WoL.
>>
>> I should have been cleared, I meant using genphy_{suspend,resume} to
>> avoid open coding the setting of the BMCR_PDOWN bit, conversely clearing
>> of that bit. Because of the locking, maybe you could introduce unlocked
>> versions of these two routines, or you acquire and release the lock
>> outside of genphy_{suspend,resume}?
>
> OK I have addressed all of the open comments and will be posting v2 shortly.
>
> I do have a question on this request.
> Are you indicating to exclusively call genphy_(suspend/resume) from within the over ridden
> routine and then take care of the WoL bits in the phy specific code?
>
> Since the genphy code is duplicated here for the BMCR value that would make the most sense
> to me. The genphy code is exported so I can call it explicitly then do the WoL
I would expect you to wrap your own calls around genphy_suspend and
genphy_resume, such your functions become:
static int dp83822_suspend(struct phy_device *phydev)
{
int value;
mutex_lock(&phydev->lock);
value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
mutex_unlock(&phydev->lock);
if (~value & DP83822_WOL_EN)
genphy_suspend(phydev);
}
static int dp83822_resume(struct phy_device *phydev)
{
int value;
genphy_resume(phydev);
mutex_lock(&phydev->lock);
value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG, value |
DP83822_WOL_CLR_INDICATION);
mutex_unlock(&phydev->lock);
return 0;
}
>
> Dan
>
> [snip]--
> ------------------
> Dan Murphy
>
--
Florian
^ permalink raw reply
* Re: [PATCH 0/3] iwlwifi: cosmetic fixes
From: Luciano Coelho @ 2017-10-04 16:21 UTC (permalink / raw)
To: Christoph Böhmwalder, johannes.berg, emmanuel.grumbach,
kvalo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20171004155700.18048-1-christoph@boehmwalder.at>
On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Fix several code style issues, some of which were reported by
> checkpatch.pl.
>
> The changes are:
> * One instance of an `int` variable being used in a boolean context,
> chaned to
> use the more appropriate `bool` type.
> * One very minor fix, removing a newline between a function
> definition and its
> associated `static` keyword
> * One fix wrapping a macro in curly braces
>
>
> Christoph Böhmwalder (3):
> wireless: iwlwifi: use bool instead of int
> wireless: iwlwifi: function definition cosmetic fix
> wireless: iwlwifi: wrap macro into braces
>
> drivers/net/wireless/intel/iwlwifi/iwl-io.c | 2 +-
> drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 16 +++++++------
> ---
> 2 files changed, 8 insertions(+), 10 deletions(-)
Sorry, but this kind of series just generates churn. Especially when 2
out of 3 patches are broken. I applied your previous patch because it
was really trivial, but I really don't want to encourage this kind of
drive-by "fixes" that only cause additional work.
I generally only accept this kind of changes when people are changing
code close or related to it.
--
Luca.
^ permalink raw reply
* Re: multi producer / multi consumer lock-free queue with ptr_ring
From: Michael S. Tsirkin @ 2017-10-04 16:23 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: LKML, Netdev, Samuel Holland, WireGuard mailing list
In-Reply-To: <CAHmME9qPYL-wzyX0Q7RG7Uk9kAeWbzNN5=0YrmbniUR2O3rSxA@mail.gmail.com>
On Wed, Oct 04, 2017 at 01:18:24PM +0200, Jason A. Donenfeld wrote:
> Hey Michael,
>
> Thanks for your work on ptr_ring.h. I'm interested in using it, but in
> a multi-producer, multi-consumer context. I realize it's been designed
> for a single-producer, single-consumer context, and thus uses a
> spinlock. I'm wondering if you'd be happy to receive patches that
> implement things in a lock-free way, in order to make the data
> structure more broadly usable.
>
> In case you're curious, this would be used for the multi-core
> algorithms in WireGuard.
>
> Thanks,
> Jason
Hi!
We'll have to look at the performance and how much code is shared.
Especially resize support might be tricky without a lock.
One interesting consideration is that currently we use spinlocks which
are very expensive but fair when contended.
And lock-free mechanisms are very rarely fair.
Further ring isn't really fair when full in the sense that
you might get the lock first but the result will only
be that you see a full ring and can not add an entry.
So maybe replacing (optionally?) a spinlock with just a trylock and fail
on contention would already speed up things for multi-consumer/producer
without the overhead and complexity of lock-free.
Or tweak spin lock to poll for a bit until going slow path
or until failure.
Or even try a bit lock.
I hope these ideas help.
One area where we definitely could share effort is adding some
micro-benchmarks under tools/. I currently (ab)use
tools/virtio/ringtest.
--
MST
^ permalink raw reply
* Re: [PATCH] net: phy: DP83822 initial driver submission
From: Dan Murphy @ 2017-10-04 16:22 UTC (permalink / raw)
To: Florian Fainelli, andrew; +Cc: netdev
In-Reply-To: <35e1541d-1cad-8d72-300f-b9c4d0c27f11@gmail.com>
Florian
On 10/04/2017 11:18 AM, Florian Fainelli wrote:
> On 10/04/2017 08:41 AM, Dan Murphy wrote:
>> Florian
>>
>> On 10/03/2017 01:31 PM, Florian Fainelli wrote:
>>> On 10/03/2017 11:03 AM, Dan Murphy wrote:
>>>> Florian
>>>>
>>>> Thanks for the review
>>>>
>>>> On 10/03/2017 12:15 PM, Florian Fainelli wrote:
>>>>>> + } else {
>>>>>> + value &= ~DP83822_WOL_SECURE_ON;
>>>>>> + }
>>>>>> +
>>>>>> + value |= (DP83822_WOL_EN | DP83822_WOL_CLR_INDICATION |
>>>>>> + DP83822_WOL_CLR_INDICATION);
>>>>>
>>>>> The extra parenthesis should not be required here.
>>>>
>>>> I did not code that in. I had to add it after Checkpatch cribbed about it.
>>>> Let me know if you want me to remove it.
>>>
>>> Let's keep those, that does not change much.
>>>
>>>>
>>>>>
>>>>>> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
>>>>>> + value);
>>>>>> + } else {
>>>>>> + value =
>>>>>> + phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
>>>>>> + value &= (~DP83822_WOL_EN);
>>>>>
>>>>> Same here, parenthesis should not be needed.
>>>>
>>>> There are three lines of code in the else. This code all needs to be excuted in the else case.
>>>> I might reformat it to read better. Lindent messed that one up.
>>>
>>> sorry, I meant to write that you don't need the parenthesis around
>>> DP83822_WOL_EN since that is just a single bit here.
>>>
>>> [snip]
>>>
>>>>>> +
>>>>>> + mutex_unlock(&phydev->lock);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static int dp83822_resume(struct phy_device *phydev)
>>>>>> +{
>>>>>> + int value;
>>>>>> +
>>>>>> + mutex_lock(&phydev->lock);
>>>>>> +
>>>>>> + value = phy_read(phydev, MII_BMCR);
>>>>>> + phy_write(phydev, MII_BMCR, value & ~BMCR_PDOWN);
>>>>>
>>>>> And genphy_resume() here as well?
>>>>
>>>> genphy_resume does not have WoL.
>>>
>>> I should have been cleared, I meant using genphy_{suspend,resume} to
>>> avoid open coding the setting of the BMCR_PDOWN bit, conversely clearing
>>> of that bit. Because of the locking, maybe you could introduce unlocked
>>> versions of these two routines, or you acquire and release the lock
>>> outside of genphy_{suspend,resume}?
>>
>> OK I have addressed all of the open comments and will be posting v2 shortly.
>>
>> I do have a question on this request.
>> Are you indicating to exclusively call genphy_(suspend/resume) from within the over ridden
>> routine and then take care of the WoL bits in the phy specific code?
>>
>> Since the genphy code is duplicated here for the BMCR value that would make the most sense
>> to me. The genphy code is exported so I can call it explicitly then do the WoL
>
> I would expect you to wrap your own calls around genphy_suspend and
> genphy_resume, such your functions become:
>
> static int dp83822_suspend(struct phy_device *phydev)
> {
> int value;
>
> mutex_lock(&phydev->lock);
> value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
> mutex_unlock(&phydev->lock);
>
> if (~value & DP83822_WOL_EN)
> genphy_suspend(phydev);
> }
>
> static int dp83822_resume(struct phy_device *phydev)
> {
> int value;
>
> genphy_resume(phydev);
>
> mutex_lock(&phydev->lock);
> value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
>
> phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG, value |
> DP83822_WOL_CLR_INDICATION);
>
> mutex_unlock(&phydev->lock);
>
> return 0;
> }
>
Great thats exactly what I did.
Dan
>>
>> Dan
>>
>> [snip]--
>> ------------------
>> Dan Murphy
>>
>
>
--
------------------
Dan Murphy
^ permalink raw reply
* Re: [PATCH 1/3] wireless: iwlwifi: use bool instead of int
From: Joe Perches @ 2017-10-04 16:26 UTC (permalink / raw)
To: Christoph Böhmwalder, johannes.berg, emmanuel.grumbach,
luciano.coelho, kvalo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20171004155700.18048-2-christoph@boehmwalder.at>
On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Change a usage of int in a boolean context to use the bool type instead, as it
> makes the intent of the function clearer and helps clarify its semantics.
>
> Also eliminate the if/else and just return the boolean result directly,
> making the code more readable.
>
> Signed-off-by: Christoph Böhmwalder <christoph@boehmwalder.at>
> ---
> drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> index b7cd813ba70f..0eb815ae97e8 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> @@ -267,14 +267,12 @@ int iwl_phy_db_set_section(struct iwl_phy_db *phy_db,
> }
> IWL_EXPORT_SYMBOL(iwl_phy_db_set_section);
>
> -static int is_valid_channel(u16 ch_id)
> +static bool is_valid_channel(u16 ch_id)
> {
> - if (ch_id <= 14 ||
> - (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> - (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> - (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1))
> - return 1;
> - return 0;
> + return (ch_id <= 14 ||
> + (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> + (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> + (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1));
> }
This might be more intelligble as separate tests
static bool is_valid_channel(u16 ch_id)
{
if (ch_id <= 14)
return true;
if ((ch_id % 4 == 0) &&
((ch_id >= 36 && ch_id <= 64) ||
(ch_id >= 100 && ch_id <= 140)))
return true;
if ((ch_id % 4 == 1) &&
(chid >= 145 && ch_id <= 165))
return true;
return false;
}
The compiler should produce the same object code.
^ permalink raw reply
* [PATCH v2 0/5] VSOCK: add sock_diag interface
From: Stefan Hajnoczi @ 2017-10-04 16:37 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Jorgen Hansen, Dexuan Cui, Stefan Hajnoczi
v2:
* Moved tests to tools/testing/vsock/. I was unable to put them in selftests/
because they require manual setup of a VMware/KVM guest.
* Moved to __vsock_in_bound/connected_table() to af_vsock.h
* Fixed local variable ordering in Patch 4
There is currently no way for userspace to query open AF_VSOCK sockets. This
means ss(8), netstat(8), and other utilities cannot display AF_VSOCK sockets.
This patch series adds the netlink sock_diag interface for AF_VSOCK. Userspace
programs sent a DUMP request including an sk_state bitmap to filter sockets
based on their state (connected, listening, etc). The vsock_diag.ko module
replies with information about matching sockets. This userspace ABI is defined
in <linux/vm_sockets_diag.h>.
The final patch adds a test suite that exercises the basic cases.
Jorgen and Dexuan: I have only tested the virtio transport but this should also
work for VMCI and Hyper-V. Please give it a shot if you have time.
Stefan Hajnoczi (5):
VSOCK: export socket tables for sock_diag interface
VSOCK: move __vsock_in_bound/connected_table() to af_vsock.h
VSOCK: use TCP state constants for sk_state
VSOCK: add sock_diag interface
VSOCK: add tools/testing/vsock/vsock_diag_test
MAINTAINERS | 3 +
net/vmw_vsock/Makefile | 3 +
tools/testing/vsock/Makefile | 9 +
include/net/af_vsock.h | 20 +-
include/uapi/linux/vm_sockets_diag.h | 33 ++
tools/testing/vsock/control.h | 13 +
tools/testing/vsock/timeout.h | 14 +
net/vmw_vsock/af_vsock.c | 66 +--
net/vmw_vsock/diag.c | 186 ++++++++
net/vmw_vsock/virtio_transport.c | 2 +-
net/vmw_vsock/virtio_transport_common.c | 22 +-
net/vmw_vsock/vmci_transport.c | 34 +-
net/vmw_vsock/vmci_transport_notify.c | 2 +-
net/vmw_vsock/vmci_transport_notify_qstate.c | 2 +-
tools/testing/vsock/control.c | 219 +++++++++
tools/testing/vsock/timeout.c | 64 +++
tools/testing/vsock/vsock_diag_test.c | 681 +++++++++++++++++++++++++++
net/vmw_vsock/Kconfig | 10 +
tools/testing/vsock/.gitignore | 2 +
tools/testing/vsock/README | 36 ++
20 files changed, 1354 insertions(+), 67 deletions(-)
create mode 100644 tools/testing/vsock/Makefile
create mode 100644 include/uapi/linux/vm_sockets_diag.h
create mode 100644 tools/testing/vsock/control.h
create mode 100644 tools/testing/vsock/timeout.h
create mode 100644 net/vmw_vsock/diag.c
create mode 100644 tools/testing/vsock/control.c
create mode 100644 tools/testing/vsock/timeout.c
create mode 100644 tools/testing/vsock/vsock_diag_test.c
create mode 100644 tools/testing/vsock/.gitignore
create mode 100644 tools/testing/vsock/README
--
2.13.6
^ permalink raw reply
* [PATCH v2 1/5] VSOCK: export socket tables for sock_diag interface
From: Stefan Hajnoczi @ 2017-10-04 16:37 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Jorgen Hansen, Dexuan Cui, Stefan Hajnoczi
In-Reply-To: <20171004163716.3964-1-stefanha@redhat.com>
The socket table symbols need to be exported from vsock.ko so that the
vsock_diag.ko module will be able to traverse sockets.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/net/af_vsock.h | 5 +++++
net/vmw_vsock/af_vsock.c | 10 ++++++----
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index f9fb566e75cf..30cba806e344 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -27,6 +27,11 @@
#define LAST_RESERVED_PORT 1023
+#define VSOCK_HASH_SIZE 251
+extern struct list_head vsock_bind_table[VSOCK_HASH_SIZE + 1];
+extern struct list_head vsock_connected_table[VSOCK_HASH_SIZE];
+extern spinlock_t vsock_table_lock;
+
#define vsock_sk(__sk) ((struct vsock_sock *)__sk)
#define sk_vsock(__vsk) (&(__vsk)->sk)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index dfc8c51e4d74..9afe4da8c67d 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -153,7 +153,6 @@ EXPORT_SYMBOL_GPL(vm_sockets_get_local_cid);
* vsock_bind_table[VSOCK_HASH_SIZE] is for unbound sockets. The hash function
* mods with VSOCK_HASH_SIZE to ensure this.
*/
-#define VSOCK_HASH_SIZE 251
#define MAX_PORT_RETRIES 24
#define VSOCK_HASH(addr) ((addr)->svm_port % VSOCK_HASH_SIZE)
@@ -168,9 +167,12 @@ EXPORT_SYMBOL_GPL(vm_sockets_get_local_cid);
#define vsock_connected_sockets_vsk(vsk) \
vsock_connected_sockets(&(vsk)->remote_addr, &(vsk)->local_addr)
-static struct list_head vsock_bind_table[VSOCK_HASH_SIZE + 1];
-static struct list_head vsock_connected_table[VSOCK_HASH_SIZE];
-static DEFINE_SPINLOCK(vsock_table_lock);
+struct list_head vsock_bind_table[VSOCK_HASH_SIZE + 1];
+EXPORT_SYMBOL_GPL(vsock_bind_table);
+struct list_head vsock_connected_table[VSOCK_HASH_SIZE];
+EXPORT_SYMBOL_GPL(vsock_connected_table);
+DEFINE_SPINLOCK(vsock_table_lock);
+EXPORT_SYMBOL_GPL(vsock_table_lock);
/* Autobind this socket to the local address if necessary. */
static int vsock_auto_bind(struct vsock_sock *vsk)
--
2.13.6
^ permalink raw reply related
* [PATCH v2 2/5] VSOCK: move __vsock_in_bound/connected_table() to af_vsock.h
From: Stefan Hajnoczi @ 2017-10-04 16:37 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Jorgen Hansen, Dexuan Cui, Stefan Hajnoczi
In-Reply-To: <20171004163716.3964-1-stefanha@redhat.com>
The vsock_diag.ko module will need to check socket table membership.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/net/af_vsock.h | 12 ++++++++++++
net/vmw_vsock/af_vsock.c | 10 ----------
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index 30cba806e344..3dd217718a2f 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -180,6 +180,18 @@ const struct vsock_transport *vsock_core_get_transport(void);
/**** UTILS ****/
+/* vsock_table_lock must be held */
+static inline bool __vsock_in_bound_table(struct vsock_sock *vsk)
+{
+ return !list_empty(&vsk->bound_table);
+}
+
+/* vsock_table_lock must be held */
+static inline bool __vsock_in_connected_table(struct vsock_sock *vsk)
+{
+ return !list_empty(&vsk->connected_table);
+}
+
void vsock_release_pending(struct sock *pending);
void vsock_add_pending(struct sock *listener, struct sock *pending);
void vsock_remove_pending(struct sock *listener, struct sock *pending);
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 9afe4da8c67d..9b179a0081b3 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -250,16 +250,6 @@ static struct sock *__vsock_find_connected_socket(struct sockaddr_vm *src,
return NULL;
}
-static bool __vsock_in_bound_table(struct vsock_sock *vsk)
-{
- return !list_empty(&vsk->bound_table);
-}
-
-static bool __vsock_in_connected_table(struct vsock_sock *vsk)
-{
- return !list_empty(&vsk->connected_table);
-}
-
static void vsock_insert_unbound(struct vsock_sock *vsk)
{
spin_lock_bh(&vsock_table_lock);
--
2.13.6
^ permalink raw reply related
* [PATCH v2 3/5] VSOCK: use TCP state constants for sk_state
From: Stefan Hajnoczi @ 2017-10-04 16:37 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Jorgen Hansen, Dexuan Cui, Stefan Hajnoczi
In-Reply-To: <20171004163716.3964-1-stefanha@redhat.com>
There are two state fields: socket->state and sock->sk_state. The
socket->state field uses SS_UNCONNECTED, SS_CONNECTED, etc while the
sock->sk_state typically uses values that match TCP state constants
(TCP_CLOSE, TCP_ESTABLISHED). AF_VSOCK does not follow this convention
and instead uses SS_* constants for both fields.
The sk_state field will be exposed to userspace through the vsock_diag
interface for ss(8), netstat(8), and other programs.
This patch switches sk_state to TCP state constants so that the meaning
of this field is consistent with other address families. Not just
AF_INET and AF_INET6 use the TCP constants, AF_UNIX and others do too.
The following mapping was used to convert the code:
SS_FREE -> TCP_CLOSE
SS_UNCONNECTED -> TCP_CLOSE
SS_CONNECTING -> TCP_SYN_SENT
SS_CONNECTED -> TCP_ESTABLISHED
SS_DISCONNECTING -> TCP_CLOSING
VSOCK_SS_LISTEN -> TCP_LISTEN
In __vsock_create() the sk_state initialization was dropped because
sock_init_data() already initializes sk_state to TCP_CLOSE.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/net/af_vsock.h | 3 --
net/vmw_vsock/af_vsock.c | 46 ++++++++++++++++------------
net/vmw_vsock/virtio_transport.c | 2 +-
net/vmw_vsock/virtio_transport_common.c | 22 ++++++-------
net/vmw_vsock/vmci_transport.c | 34 ++++++++++----------
net/vmw_vsock/vmci_transport_notify.c | 2 +-
net/vmw_vsock/vmci_transport_notify_qstate.c | 2 +-
7 files changed, 58 insertions(+), 53 deletions(-)
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index 3dd217718a2f..9324ac2d9ff2 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -22,9 +22,6 @@
#include "vsock_addr.h"
-/* vsock-specific sock->sk_state constants */
-#define VSOCK_SS_LISTEN 255
-
#define LAST_RESERVED_PORT 1023
#define VSOCK_HASH_SIZE 251
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 9b179a0081b3..98359c19522f 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -36,7 +36,7 @@
* not support simultaneous connects (two "client" sockets connecting).
*
* - "Server" sockets are referred to as listener sockets throughout this
- * implementation because they are in the VSOCK_SS_LISTEN state. When a
+ * implementation because they are in the TCP_LISTEN state. When a
* connection request is received (the second kind of socket mentioned above),
* we create a new socket and refer to it as a pending socket. These pending
* sockets are placed on the pending connection list of the listener socket.
@@ -82,6 +82,15 @@
* argument, we must ensure the reference count is increased to ensure the
* socket isn't freed before the function is run; the deferred function will
* then drop the reference.
+ *
+ * - sk->sk_state uses the TCP state constants because they are widely used by
+ * other address families and exposed to userspace tools like ss(8):
+ *
+ * TCP_CLOSE - unconnected
+ * TCP_SYN_SENT - connecting
+ * TCP_ESTABLISHED - connected
+ * TCP_CLOSING - disconnecting
+ * TCP_LISTEN - listening
*/
#include <linux/types.h>
@@ -477,7 +486,7 @@ void vsock_pending_work(struct work_struct *work)
if (vsock_in_connected_table(vsk))
vsock_remove_connected(vsk);
- sk->sk_state = SS_FREE;
+ sk->sk_state = TCP_CLOSE;
out:
release_sock(sk);
@@ -617,7 +626,6 @@ struct sock *__vsock_create(struct net *net,
sk->sk_destruct = vsock_sk_destruct;
sk->sk_backlog_rcv = vsock_queue_rcv_skb;
- sk->sk_state = 0;
sock_reset_flag(sk, SOCK_DONE);
INIT_LIST_HEAD(&vsk->bound_table);
@@ -891,7 +899,7 @@ static unsigned int vsock_poll(struct file *file, struct socket *sock,
/* Listening sockets that have connections in their accept
* queue can be read.
*/
- if (sk->sk_state == VSOCK_SS_LISTEN
+ if (sk->sk_state == TCP_LISTEN
&& !vsock_is_accept_queue_empty(sk))
mask |= POLLIN | POLLRDNORM;
@@ -920,7 +928,7 @@ static unsigned int vsock_poll(struct file *file, struct socket *sock,
}
/* Connected sockets that can produce data can be written. */
- if (sk->sk_state == SS_CONNECTED) {
+ if (sk->sk_state == TCP_ESTABLISHED) {
if (!(sk->sk_shutdown & SEND_SHUTDOWN)) {
bool space_avail_now = false;
int ret = transport->notify_poll_out(
@@ -942,7 +950,7 @@ static unsigned int vsock_poll(struct file *file, struct socket *sock,
* POLLOUT|POLLWRNORM when peer is closed and nothing to read,
* but local send is not shutdown.
*/
- if (sk->sk_state == SS_UNCONNECTED) {
+ if (sk->sk_state == TCP_CLOSE) {
if (!(sk->sk_shutdown & SEND_SHUTDOWN))
mask |= POLLOUT | POLLWRNORM;
@@ -1112,9 +1120,9 @@ static void vsock_connect_timeout(struct work_struct *work)
sk = sk_vsock(vsk);
lock_sock(sk);
- if (sk->sk_state == SS_CONNECTING &&
+ if (sk->sk_state == TCP_SYN_SENT &&
(sk->sk_shutdown != SHUTDOWN_MASK)) {
- sk->sk_state = SS_UNCONNECTED;
+ sk->sk_state = TCP_CLOSE;
sk->sk_err = ETIMEDOUT;
sk->sk_error_report(sk);
cancel = 1;
@@ -1160,7 +1168,7 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
err = -EALREADY;
break;
default:
- if ((sk->sk_state == VSOCK_SS_LISTEN) ||
+ if ((sk->sk_state == TCP_LISTEN) ||
vsock_addr_cast(addr, addr_len, &remote_addr) != 0) {
err = -EINVAL;
goto out;
@@ -1183,7 +1191,7 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
if (err)
goto out;
- sk->sk_state = SS_CONNECTING;
+ sk->sk_state = TCP_SYN_SENT;
err = transport->connect(vsk);
if (err < 0)
@@ -1203,7 +1211,7 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
timeout = vsk->connect_timeout;
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
- while (sk->sk_state != SS_CONNECTED && sk->sk_err == 0) {
+ while (sk->sk_state != TCP_ESTABLISHED && sk->sk_err == 0) {
if (flags & O_NONBLOCK) {
/* If we're not going to block, we schedule a timeout
* function to generate a timeout on the connection
@@ -1226,13 +1234,13 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
if (signal_pending(current)) {
err = sock_intr_errno(timeout);
- sk->sk_state = SS_UNCONNECTED;
+ sk->sk_state = TCP_CLOSE;
sock->state = SS_UNCONNECTED;
vsock_transport_cancel_pkt(vsk);
goto out_wait;
} else if (timeout == 0) {
err = -ETIMEDOUT;
- sk->sk_state = SS_UNCONNECTED;
+ sk->sk_state = TCP_CLOSE;
sock->state = SS_UNCONNECTED;
vsock_transport_cancel_pkt(vsk);
goto out_wait;
@@ -1243,7 +1251,7 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
if (sk->sk_err) {
err = -sk->sk_err;
- sk->sk_state = SS_UNCONNECTED;
+ sk->sk_state = TCP_CLOSE;
sock->state = SS_UNCONNECTED;
} else {
err = 0;
@@ -1276,7 +1284,7 @@ static int vsock_accept(struct socket *sock, struct socket *newsock, int flags,
goto out;
}
- if (listener->sk_state != VSOCK_SS_LISTEN) {
+ if (listener->sk_state != TCP_LISTEN) {
err = -EINVAL;
goto out;
}
@@ -1366,7 +1374,7 @@ static int vsock_listen(struct socket *sock, int backlog)
}
sk->sk_max_ack_backlog = backlog;
- sk->sk_state = VSOCK_SS_LISTEN;
+ sk->sk_state = TCP_LISTEN;
err = 0;
@@ -1546,7 +1554,7 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
/* Callers should not provide a destination with stream sockets. */
if (msg->msg_namelen) {
- err = sk->sk_state == SS_CONNECTED ? -EISCONN : -EOPNOTSUPP;
+ err = sk->sk_state == TCP_ESTABLISHED ? -EISCONN : -EOPNOTSUPP;
goto out;
}
@@ -1557,7 +1565,7 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
goto out;
}
- if (sk->sk_state != SS_CONNECTED ||
+ if (sk->sk_state != TCP_ESTABLISHED ||
!vsock_addr_bound(&vsk->local_addr)) {
err = -ENOTCONN;
goto out;
@@ -1681,7 +1689,7 @@ vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
lock_sock(sk);
- if (sk->sk_state != SS_CONNECTED) {
+ if (sk->sk_state != TCP_ESTABLISHED) {
/* Recvmsg is supposed to return 0 if a peer performs an
* orderly shutdown. Differentiate between that case and when a
* peer has not connected or a local shutdown occured with the
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 403d86e80162..8e03bd3f3668 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -414,7 +414,7 @@ static void virtio_vsock_event_fill(struct virtio_vsock *vsock)
static void virtio_vsock_reset_sock(struct sock *sk)
{
lock_sock(sk);
- sk->sk_state = SS_UNCONNECTED;
+ sk->sk_state = TCP_CLOSE;
sk->sk_err = ECONNRESET;
sk->sk_error_report(sk);
release_sock(sk);
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index edba7ab97563..3ae3a33da70b 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -708,7 +708,7 @@ static void virtio_transport_do_close(struct vsock_sock *vsk,
sock_set_flag(sk, SOCK_DONE);
vsk->peer_shutdown = SHUTDOWN_MASK;
if (vsock_stream_has_data(vsk) <= 0)
- sk->sk_state = SS_DISCONNECTING;
+ sk->sk_state = TCP_CLOSING;
sk->sk_state_change(sk);
if (vsk->close_work_scheduled &&
@@ -748,8 +748,8 @@ static bool virtio_transport_close(struct vsock_sock *vsk)
{
struct sock *sk = &vsk->sk;
- if (!(sk->sk_state == SS_CONNECTED ||
- sk->sk_state == SS_DISCONNECTING))
+ if (!(sk->sk_state == TCP_ESTABLISHED ||
+ sk->sk_state == TCP_CLOSING))
return true;
/* Already received SHUTDOWN from peer, reply with RST */
@@ -801,7 +801,7 @@ virtio_transport_recv_connecting(struct sock *sk,
switch (le16_to_cpu(pkt->hdr.op)) {
case VIRTIO_VSOCK_OP_RESPONSE:
- sk->sk_state = SS_CONNECTED;
+ sk->sk_state = TCP_ESTABLISHED;
sk->sk_socket->state = SS_CONNECTED;
vsock_insert_connected(vsk);
sk->sk_state_change(sk);
@@ -821,7 +821,7 @@ virtio_transport_recv_connecting(struct sock *sk,
destroy:
virtio_transport_reset(vsk, pkt);
- sk->sk_state = SS_UNCONNECTED;
+ sk->sk_state = TCP_CLOSE;
sk->sk_err = skerr;
sk->sk_error_report(sk);
return err;
@@ -857,7 +857,7 @@ virtio_transport_recv_connected(struct sock *sk,
vsk->peer_shutdown |= SEND_SHUTDOWN;
if (vsk->peer_shutdown == SHUTDOWN_MASK &&
vsock_stream_has_data(vsk) <= 0)
- sk->sk_state = SS_DISCONNECTING;
+ sk->sk_state = TCP_CLOSING;
if (le32_to_cpu(pkt->hdr.flags))
sk->sk_state_change(sk);
break;
@@ -928,7 +928,7 @@ virtio_transport_recv_listen(struct sock *sk, struct virtio_vsock_pkt *pkt)
lock_sock_nested(child, SINGLE_DEPTH_NESTING);
- child->sk_state = SS_CONNECTED;
+ child->sk_state = TCP_ESTABLISHED;
vchild = vsock_sk(child);
vsock_addr_init(&vchild->local_addr, le64_to_cpu(pkt->hdr.dst_cid),
@@ -1016,18 +1016,18 @@ void virtio_transport_recv_pkt(struct virtio_vsock_pkt *pkt)
sk->sk_write_space(sk);
switch (sk->sk_state) {
- case VSOCK_SS_LISTEN:
+ case TCP_LISTEN:
virtio_transport_recv_listen(sk, pkt);
virtio_transport_free_pkt(pkt);
break;
- case SS_CONNECTING:
+ case TCP_SYN_SENT:
virtio_transport_recv_connecting(sk, pkt);
virtio_transport_free_pkt(pkt);
break;
- case SS_CONNECTED:
+ case TCP_ESTABLISHED:
virtio_transport_recv_connected(sk, pkt);
break;
- case SS_DISCONNECTING:
+ case TCP_CLOSING:
virtio_transport_recv_disconnecting(sk, pkt);
virtio_transport_free_pkt(pkt);
break;
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index 10ae7823a19d..9cb3a6c780aa 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -743,7 +743,7 @@ static int vmci_transport_recv_stream_cb(void *data, struct vmci_datagram *dg)
/* The local context ID may be out of date, update it. */
vsk->local_addr.svm_cid = dst.svm_cid;
- if (sk->sk_state == SS_CONNECTED)
+ if (sk->sk_state == TCP_ESTABLISHED)
vmci_trans(vsk)->notify_ops->handle_notify_pkt(
sk, pkt, true, &dst, &src,
&bh_process_pkt);
@@ -801,7 +801,9 @@ static void vmci_transport_handle_detach(struct sock *sk)
* left in our consume queue.
*/
if (vsock_stream_has_data(vsk) <= 0) {
- if (sk->sk_state == SS_CONNECTING) {
+ sk->sk_state = TCP_CLOSE;
+
+ if (sk->sk_state == TCP_SYN_SENT) {
/* The peer may detach from a queue pair while
* we are still in the connecting state, i.e.,
* if the peer VM is killed after attaching to
@@ -810,12 +812,10 @@ static void vmci_transport_handle_detach(struct sock *sk)
* event like a reset.
*/
- sk->sk_state = SS_UNCONNECTED;
sk->sk_err = ECONNRESET;
sk->sk_error_report(sk);
return;
}
- sk->sk_state = SS_UNCONNECTED;
}
sk->sk_state_change(sk);
}
@@ -883,17 +883,17 @@ static void vmci_transport_recv_pkt_work(struct work_struct *work)
vsock_sk(sk)->local_addr.svm_cid = pkt->dg.dst.context;
switch (sk->sk_state) {
- case VSOCK_SS_LISTEN:
+ case TCP_LISTEN:
vmci_transport_recv_listen(sk, pkt);
break;
- case SS_CONNECTING:
+ case TCP_SYN_SENT:
/* Processing of pending connections for servers goes through
* the listening socket, so see vmci_transport_recv_listen()
* for that path.
*/
vmci_transport_recv_connecting_client(sk, pkt);
break;
- case SS_CONNECTED:
+ case TCP_ESTABLISHED:
vmci_transport_recv_connected(sk, pkt);
break;
default:
@@ -942,7 +942,7 @@ static int vmci_transport_recv_listen(struct sock *sk,
vsock_sk(pending)->local_addr.svm_cid = pkt->dg.dst.context;
switch (pending->sk_state) {
- case SS_CONNECTING:
+ case TCP_SYN_SENT:
err = vmci_transport_recv_connecting_server(sk,
pending,
pkt);
@@ -1072,7 +1072,7 @@ static int vmci_transport_recv_listen(struct sock *sk,
vsock_add_pending(sk, pending);
sk->sk_ack_backlog++;
- pending->sk_state = SS_CONNECTING;
+ pending->sk_state = TCP_SYN_SENT;
vmci_trans(vpending)->produce_size =
vmci_trans(vpending)->consume_size = qp_size;
vmci_trans(vpending)->queue_pair_size = qp_size;
@@ -1197,11 +1197,11 @@ vmci_transport_recv_connecting_server(struct sock *listener,
* the socket will be valid until it is removed from the queue.
*
* If we fail sending the attach below, we remove the socket from the
- * connected list and move the socket to SS_UNCONNECTED before
+ * connected list and move the socket to TCP_CLOSE before
* releasing the lock, so a pending slow path processing of an incoming
* packet will not see the socket in the connected state in that case.
*/
- pending->sk_state = SS_CONNECTED;
+ pending->sk_state = TCP_ESTABLISHED;
vsock_insert_connected(vpending);
@@ -1232,7 +1232,7 @@ vmci_transport_recv_connecting_server(struct sock *listener,
destroy:
pending->sk_err = skerr;
- pending->sk_state = SS_UNCONNECTED;
+ pending->sk_state = TCP_CLOSE;
/* As long as we drop our reference, all necessary cleanup will handle
* when the cleanup function drops its reference and our destruct
* implementation is called. Note that since the listen handler will
@@ -1270,7 +1270,7 @@ vmci_transport_recv_connecting_client(struct sock *sk,
* accounting (it can already be found since it's in the bound
* table).
*/
- sk->sk_state = SS_CONNECTED;
+ sk->sk_state = TCP_ESTABLISHED;
sk->sk_socket->state = SS_CONNECTED;
vsock_insert_connected(vsk);
sk->sk_state_change(sk);
@@ -1338,7 +1338,7 @@ vmci_transport_recv_connecting_client(struct sock *sk,
destroy:
vmci_transport_send_reset(sk, pkt);
- sk->sk_state = SS_UNCONNECTED;
+ sk->sk_state = TCP_CLOSE;
sk->sk_err = skerr;
sk->sk_error_report(sk);
return err;
@@ -1526,7 +1526,7 @@ static int vmci_transport_recv_connected(struct sock *sk,
sock_set_flag(sk, SOCK_DONE);
vsk->peer_shutdown = SHUTDOWN_MASK;
if (vsock_stream_has_data(vsk) <= 0)
- sk->sk_state = SS_DISCONNECTING;
+ sk->sk_state = TCP_CLOSING;
sk->sk_state_change(sk);
break;
@@ -1790,7 +1790,7 @@ static int vmci_transport_connect(struct vsock_sock *vsk)
err = vmci_transport_send_conn_request(
sk, vmci_trans(vsk)->queue_pair_size);
if (err < 0) {
- sk->sk_state = SS_UNCONNECTED;
+ sk->sk_state = TCP_CLOSE;
return err;
}
} else {
@@ -1800,7 +1800,7 @@ static int vmci_transport_connect(struct vsock_sock *vsk)
sk, vmci_trans(vsk)->queue_pair_size,
supported_proto_versions);
if (err < 0) {
- sk->sk_state = SS_UNCONNECTED;
+ sk->sk_state = TCP_CLOSE;
return err;
}
diff --git a/net/vmw_vsock/vmci_transport_notify.c b/net/vmw_vsock/vmci_transport_notify.c
index 1406db4d97d1..41fb427f150a 100644
--- a/net/vmw_vsock/vmci_transport_notify.c
+++ b/net/vmw_vsock/vmci_transport_notify.c
@@ -355,7 +355,7 @@ vmci_transport_notify_pkt_poll_in(struct sock *sk,
* queue. Ask for notifications when there is something to
* read.
*/
- if (sk->sk_state == SS_CONNECTED) {
+ if (sk->sk_state == TCP_ESTABLISHED) {
if (!send_waiting_read(sk, 1))
return -1;
diff --git a/net/vmw_vsock/vmci_transport_notify_qstate.c b/net/vmw_vsock/vmci_transport_notify_qstate.c
index f3a0afc46208..0cc84f2bb05e 100644
--- a/net/vmw_vsock/vmci_transport_notify_qstate.c
+++ b/net/vmw_vsock/vmci_transport_notify_qstate.c
@@ -176,7 +176,7 @@ vmci_transport_notify_pkt_poll_in(struct sock *sk,
* queue. Ask for notifications when there is something to
* read.
*/
- if (sk->sk_state == SS_CONNECTED)
+ if (sk->sk_state == TCP_ESTABLISHED)
vsock_block_update_write_window(sk);
*data_ready_now = false;
}
--
2.13.6
^ permalink raw reply related
* [PATCH v2 4/5] VSOCK: add sock_diag interface
From: Stefan Hajnoczi @ 2017-10-04 16:37 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Jorgen Hansen, Dexuan Cui, Stefan Hajnoczi
In-Reply-To: <20171004163716.3964-1-stefanha@redhat.com>
This patch adds the sock_diag interface for querying sockets from
userspace. Tools like ss(8) and netstat(8) can use this interface to
list open sockets.
The userspace ABI is defined in <linux/vm_sockets_diag.h> and includes
netlink request and response structs. The request can query sockets
based on their sk_state (e.g. listening sockets only) and the response
contains socket information fields including the local/remote addresses,
inode number, etc.
This patch does not dump VMCI pending sockets because I have only tested
the virtio transport, which does not use pending sockets. Support can
be added later by extending vsock_diag_dump() if needed by VMCI users.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
MAINTAINERS | 2 +
net/vmw_vsock/Makefile | 3 +
include/uapi/linux/vm_sockets_diag.h | 33 +++++++
net/vmw_vsock/diag.c | 186 +++++++++++++++++++++++++++++++++++
net/vmw_vsock/Kconfig | 10 ++
5 files changed, 234 insertions(+)
create mode 100644 include/uapi/linux/vm_sockets_diag.h
create mode 100644 net/vmw_vsock/diag.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 1c3feffb1c1c..200dac93f34b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13975,6 +13975,8 @@ S: Maintained
F: include/linux/virtio_vsock.h
F: include/uapi/linux/virtio_vsock.h
F: include/uapi/linux/vsockmon.h
+F: include/uapi/linux/vm_sockets_diag.h
+F: net/vmw_vsock/diag.c
F: net/vmw_vsock/af_vsock_tap.c
F: net/vmw_vsock/virtio_transport_common.c
F: net/vmw_vsock/virtio_transport.c
diff --git a/net/vmw_vsock/Makefile b/net/vmw_vsock/Makefile
index 09fc2eb29dc8..e5dbf153aff0 100644
--- a/net/vmw_vsock/Makefile
+++ b/net/vmw_vsock/Makefile
@@ -1,10 +1,13 @@
obj-$(CONFIG_VSOCKETS) += vsock.o
+obj-$(CONFIG_VSOCKETS_DIAG) += vsock_diag.o
obj-$(CONFIG_VMWARE_VMCI_VSOCKETS) += vmw_vsock_vmci_transport.o
obj-$(CONFIG_VIRTIO_VSOCKETS) += vmw_vsock_virtio_transport.o
obj-$(CONFIG_VIRTIO_VSOCKETS_COMMON) += vmw_vsock_virtio_transport_common.o
vsock-y += af_vsock.o af_vsock_tap.o vsock_addr.o
+vsock_diag-y += diag.o
+
vmw_vsock_vmci_transport-y += vmci_transport.o vmci_transport_notify.o \
vmci_transport_notify_qstate.o
diff --git a/include/uapi/linux/vm_sockets_diag.h b/include/uapi/linux/vm_sockets_diag.h
new file mode 100644
index 000000000000..14cd7dc5a187
--- /dev/null
+++ b/include/uapi/linux/vm_sockets_diag.h
@@ -0,0 +1,33 @@
+/* AF_VSOCK sock_diag(7) interface for querying open sockets */
+
+#ifndef _UAPI__VM_SOCKETS_DIAG_H__
+#define _UAPI__VM_SOCKETS_DIAG_H__
+
+#include <linux/types.h>
+
+/* Request */
+struct vsock_diag_req {
+ __u8 sdiag_family; /* must be AF_VSOCK */
+ __u8 sdiag_protocol; /* must be 0 */
+ __u16 pad; /* must be 0 */
+ __u32 vdiag_states; /* query bitmap (e.g. 1 << TCP_LISTEN) */
+ __u32 vdiag_ino; /* must be 0 (reserved) */
+ __u32 vdiag_show; /* must be 0 (reserved) */
+ __u32 vdiag_cookie[2];
+};
+
+/* Response */
+struct vsock_diag_msg {
+ __u8 vdiag_family; /* AF_VSOCK */
+ __u8 vdiag_type; /* SOCK_STREAM or SOCK_DGRAM */
+ __u8 vdiag_state; /* sk_state (e.g. TCP_LISTEN) */
+ __u8 vdiag_shutdown; /* local RCV_SHUTDOWN | SEND_SHUTDOWN */
+ __u32 vdiag_src_cid;
+ __u32 vdiag_src_port;
+ __u32 vdiag_dst_cid;
+ __u32 vdiag_dst_port;
+ __u32 vdiag_ino;
+ __u32 vdiag_cookie[2];
+};
+
+#endif /* _UAPI__VM_SOCKETS_DIAG_H__ */
diff --git a/net/vmw_vsock/diag.c b/net/vmw_vsock/diag.c
new file mode 100644
index 000000000000..31b567652250
--- /dev/null
+++ b/net/vmw_vsock/diag.c
@@ -0,0 +1,186 @@
+/*
+ * vsock sock_diag(7) module
+ *
+ * Copyright (C) 2017 Red Hat, Inc.
+ * Author: Stefan Hajnoczi <stefanha@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation version 2 and no later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/module.h>
+#include <linux/sock_diag.h>
+#include <linux/vm_sockets_diag.h>
+#include <net/af_vsock.h>
+
+static int sk_diag_fill(struct sock *sk, struct sk_buff *skb,
+ u32 portid, u32 seq, u32 flags)
+{
+ struct vsock_sock *vsk = vsock_sk(sk);
+ struct vsock_diag_msg *rep;
+ struct nlmsghdr *nlh;
+
+ nlh = nlmsg_put(skb, portid, seq, SOCK_DIAG_BY_FAMILY, sizeof(*rep),
+ flags);
+ if (!nlh)
+ return -EMSGSIZE;
+
+ rep = nlmsg_data(nlh);
+ rep->vdiag_family = AF_VSOCK;
+
+ /* Lock order dictates that sk_lock is acquired before
+ * vsock_table_lock, so we cannot lock here. Simply don't take
+ * sk_lock; sk is guaranteed to stay alive since vsock_table_lock is
+ * held.
+ */
+ rep->vdiag_type = sk->sk_type;
+ rep->vdiag_state = sk->sk_state;
+ rep->vdiag_shutdown = sk->sk_shutdown;
+ rep->vdiag_src_cid = vsk->local_addr.svm_cid;
+ rep->vdiag_src_port = vsk->local_addr.svm_port;
+ rep->vdiag_dst_cid = vsk->remote_addr.svm_cid;
+ rep->vdiag_dst_port = vsk->remote_addr.svm_port;
+ rep->vdiag_ino = sock_i_ino(sk);
+
+ sock_diag_save_cookie(sk, rep->vdiag_cookie);
+
+ return 0;
+}
+
+static int vsock_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ struct vsock_diag_req *req;
+ struct vsock_sock *vsk;
+ unsigned int bucket;
+ unsigned int last_i;
+ unsigned int table;
+ struct net *net;
+ unsigned int i;
+
+ req = nlmsg_data(cb->nlh);
+ net = sock_net(skb->sk);
+
+ /* State saved between calls: */
+ table = cb->args[0];
+ bucket = cb->args[1];
+ i = last_i = cb->args[2];
+
+ /* TODO VMCI pending sockets? */
+
+ spin_lock_bh(&vsock_table_lock);
+
+ /* Bind table (locally created sockets) */
+ if (table == 0) {
+ while (bucket < ARRAY_SIZE(vsock_bind_table)) {
+ struct list_head *head = &vsock_bind_table[bucket];
+
+ i = 0;
+ list_for_each_entry(vsk, head, bound_table) {
+ struct sock *sk = sk_vsock(vsk);
+
+ if (!net_eq(sock_net(sk), net))
+ continue;
+ if (i < last_i)
+ goto next_bind;
+ if (!(req->vdiag_states & (1 << sk->sk_state)))
+ goto next_bind;
+ if (sk_diag_fill(sk, skb,
+ NETLINK_CB(cb->skb).portid,
+ cb->nlh->nlmsg_seq,
+ NLM_F_MULTI) < 0)
+ goto done;
+next_bind:
+ i++;
+ }
+ last_i = 0;
+ bucket++;
+ }
+
+ table++;
+ bucket = 0;
+ }
+
+ /* Connected table (accepted connections) */
+ while (bucket < ARRAY_SIZE(vsock_connected_table)) {
+ struct list_head *head = &vsock_connected_table[bucket];
+
+ i = 0;
+ list_for_each_entry(vsk, head, connected_table) {
+ struct sock *sk = sk_vsock(vsk);
+
+ /* Skip sockets we've already seen above */
+ if (__vsock_in_bound_table(vsk))
+ continue;
+
+ if (!net_eq(sock_net(sk), net))
+ continue;
+ if (i < last_i)
+ goto next_connected;
+ if (!(req->vdiag_states & (1 << sk->sk_state)))
+ goto next_connected;
+ if (sk_diag_fill(sk, skb,
+ NETLINK_CB(cb->skb).portid,
+ cb->nlh->nlmsg_seq,
+ NLM_F_MULTI) < 0)
+ goto done;
+next_connected:
+ i++;
+ }
+ last_i = 0;
+ bucket++;
+ }
+
+done:
+ spin_unlock_bh(&vsock_table_lock);
+
+ cb->args[0] = table;
+ cb->args[1] = bucket;
+ cb->args[2] = i;
+
+ return skb->len;
+}
+
+static int vsock_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
+{
+ int hdrlen = sizeof(struct vsock_diag_req);
+ struct net *net = sock_net(skb->sk);
+
+ if (nlmsg_len(h) < hdrlen)
+ return -EINVAL;
+
+ if (h->nlmsg_flags & NLM_F_DUMP) {
+ struct netlink_dump_control c = {
+ .dump = vsock_diag_dump,
+ };
+ return netlink_dump_start(net->diag_nlsk, skb, h, &c);
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static const struct sock_diag_handler vsock_diag_handler = {
+ .family = AF_VSOCK,
+ .dump = vsock_diag_handler_dump,
+};
+
+static int __init vsock_diag_init(void)
+{
+ return sock_diag_register(&vsock_diag_handler);
+}
+
+static void __exit vsock_diag_exit(void)
+{
+ sock_diag_unregister(&vsock_diag_handler);
+}
+
+module_init(vsock_diag_init);
+module_exit(vsock_diag_exit);
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_NETLINK, NETLINK_SOCK_DIAG,
+ 40 /* AF_VSOCK */);
diff --git a/net/vmw_vsock/Kconfig b/net/vmw_vsock/Kconfig
index 8831e7c42167..829cb7c8f14c 100644
--- a/net/vmw_vsock/Kconfig
+++ b/net/vmw_vsock/Kconfig
@@ -15,6 +15,16 @@ config VSOCKETS
To compile this driver as a module, choose M here: the module
will be called vsock. If unsure, say N.
+config VSOCKETS_DIAG
+ tristate "Virtual Sockets monitoring interface"
+ depends on VSOCKETS
+ default y
+ help
+ Support for PF_VSOCK sockets monitoring interface used by the ss tool.
+ If unsure, say Y.
+
+ Enable this module so userspace applications can query open sockets.
+
config VMWARE_VMCI_VSOCKETS
tristate "VMware VMCI transport for Virtual Sockets"
depends on VSOCKETS && VMWARE_VMCI
--
2.13.6
^ permalink raw reply related
* [PATCH v2 5/5] VSOCK: add tools/testing/vsock/vsock_diag_test
From: Stefan Hajnoczi @ 2017-10-04 16:37 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Jorgen Hansen, Dexuan Cui, Stefan Hajnoczi
In-Reply-To: <20171004163716.3964-1-stefanha@redhat.com>
This patch adds tests for the vsock_diag.ko module.
These tests are not self-tests because they require manual set up of a
KVM or VMware guest. Please see tools/testing/vsock/README for
instructions.
The control.h and timeout.h infrastructure can be used for additional
AF_VSOCK tests in the future.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
MAINTAINERS | 1 +
tools/testing/vsock/Makefile | 9 +
tools/testing/vsock/control.h | 13 +
tools/testing/vsock/timeout.h | 14 +
tools/testing/vsock/control.c | 219 +++++++++++
tools/testing/vsock/timeout.c | 64 ++++
tools/testing/vsock/vsock_diag_test.c | 681 ++++++++++++++++++++++++++++++++++
tools/testing/vsock/.gitignore | 2 +
tools/testing/vsock/README | 36 ++
9 files changed, 1039 insertions(+)
create mode 100644 tools/testing/vsock/Makefile
create mode 100644 tools/testing/vsock/control.h
create mode 100644 tools/testing/vsock/timeout.h
create mode 100644 tools/testing/vsock/control.c
create mode 100644 tools/testing/vsock/timeout.c
create mode 100644 tools/testing/vsock/vsock_diag_test.c
create mode 100644 tools/testing/vsock/.gitignore
create mode 100644 tools/testing/vsock/README
diff --git a/MAINTAINERS b/MAINTAINERS
index 200dac93f34b..76b9fa587bfa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13983,6 +13983,7 @@ F: net/vmw_vsock/virtio_transport.c
F: drivers/net/vsockmon.c
F: drivers/vhost/vsock.c
F: drivers/vhost/vsock.h
+F: tools/testing/vsock/
VIRTIO CONSOLE DRIVER
M: Amit Shah <amit@kernel.org>
diff --git a/tools/testing/vsock/Makefile b/tools/testing/vsock/Makefile
new file mode 100644
index 000000000000..66ba0924194d
--- /dev/null
+++ b/tools/testing/vsock/Makefile
@@ -0,0 +1,9 @@
+all: test
+test: vsock_diag_test
+vsock_diag_test: vsock_diag_test.o timeout.o control.o
+
+CFLAGS += -g -O2 -Werror -Wall -I. -I../../include/uapi -I../../include -Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE -D_GNU_SOURCE
+.PHONY: all test clean
+clean:
+ ${RM} *.o *.d vsock_diag_test
+-include *.d
diff --git a/tools/testing/vsock/control.h b/tools/testing/vsock/control.h
new file mode 100644
index 000000000000..54a07efd267c
--- /dev/null
+++ b/tools/testing/vsock/control.h
@@ -0,0 +1,13 @@
+#ifndef CONTROL_H
+#define CONTROL_H
+
+#include <stdbool.h>
+
+void control_init(const char *control_host, const char *control_port,
+ bool server);
+void control_cleanup(void);
+void control_writeln(const char *str);
+char *control_readln(void);
+void control_expectln(const char *str);
+
+#endif /* CONTROL_H */
diff --git a/tools/testing/vsock/timeout.h b/tools/testing/vsock/timeout.h
new file mode 100644
index 000000000000..77db9ce9860a
--- /dev/null
+++ b/tools/testing/vsock/timeout.h
@@ -0,0 +1,14 @@
+#ifndef TIMEOUT_H
+#define TIMEOUT_H
+
+enum {
+ /* Default timeout */
+ TIMEOUT = 10 /* seconds */
+};
+
+void sigalrm(int signo);
+void timeout_begin(unsigned int seconds);
+void timeout_check(const char *operation);
+void timeout_end(void);
+
+#endif /* TIMEOUT_H */
diff --git a/tools/testing/vsock/control.c b/tools/testing/vsock/control.c
new file mode 100644
index 000000000000..90fd47f0e422
--- /dev/null
+++ b/tools/testing/vsock/control.c
@@ -0,0 +1,219 @@
+/* Control socket for client/server test execution
+ *
+ * Copyright (C) 2017 Red Hat, Inc.
+ *
+ * Author: Stefan Hajnoczi <stefanha@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+/* The client and server may need to coordinate to avoid race conditions like
+ * the client attempting to connect to a socket that the server is not
+ * listening on yet. The control socket offers a communications channel for
+ * such coordination tasks.
+ *
+ * If the client calls control_expectln("LISTENING"), then it will block until
+ * the server calls control_writeln("LISTENING"). This provides a simple
+ * mechanism for coordinating between the client and the server.
+ */
+
+#include <errno.h>
+#include <netdb.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+
+#include "timeout.h"
+#include "control.h"
+
+static int control_fd = -1;
+
+/* Open the control socket, either in server or client mode */
+void control_init(const char *control_host,
+ const char *control_port,
+ bool server)
+{
+ struct addrinfo hints = {
+ .ai_socktype = SOCK_STREAM,
+ };
+ struct addrinfo *result = NULL;
+ struct addrinfo *ai;
+ int ret;
+
+ ret = getaddrinfo(control_host, control_port, &hints, &result);
+ if (ret != 0) {
+ fprintf(stderr, "%s\n", gai_strerror(ret));
+ exit(EXIT_FAILURE);
+ }
+
+ for (ai = result; ai; ai = ai->ai_next) {
+ int fd;
+ int val = 1;
+
+ fd = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol);
+ if (fd < 0)
+ continue;
+
+ if (!server) {
+ if (connect(fd, ai->ai_addr, ai->ai_addrlen) < 0)
+ goto next;
+ control_fd = fd;
+ printf("Control socket connected to %s:%s.\n",
+ control_host, control_port);
+ break;
+ }
+
+ if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
+ &val, sizeof(val)) < 0) {
+ perror("setsockopt");
+ exit(EXIT_FAILURE);
+ }
+
+ if (bind(fd, ai->ai_addr, ai->ai_addrlen) < 0)
+ goto next;
+ if (listen(fd, 1) < 0)
+ goto next;
+
+ printf("Control socket listening on %s:%s\n",
+ control_host, control_port);
+ fflush(stdout);
+
+ control_fd = accept(fd, NULL, 0);
+ close(fd);
+
+ if (control_fd < 0) {
+ perror("accept");
+ exit(EXIT_FAILURE);
+ }
+ printf("Control socket connection accepted...\n");
+ break;
+
+next:
+ close(fd);
+ }
+
+ if (control_fd < 0) {
+ fprintf(stderr, "Control socket initialization failed. Invalid address %s:%s?\n",
+ control_host, control_port);
+ exit(EXIT_FAILURE);
+ }
+
+ freeaddrinfo(result);
+}
+
+/* Free resources */
+void control_cleanup(void)
+{
+ close(control_fd);
+ control_fd = -1;
+}
+
+/* Write a line to the control socket */
+void control_writeln(const char *str)
+{
+ ssize_t len = strlen(str);
+ ssize_t ret;
+
+ timeout_begin(TIMEOUT);
+
+ do {
+ ret = send(control_fd, str, len, MSG_MORE);
+ timeout_check("send");
+ } while (ret < 0 && errno == EINTR);
+
+ if (ret != len) {
+ perror("send");
+ exit(EXIT_FAILURE);
+ }
+
+ do {
+ ret = send(control_fd, "\n", 1, 0);
+ timeout_check("send");
+ } while (ret < 0 && errno == EINTR);
+
+ if (ret != 1) {
+ perror("send");
+ exit(EXIT_FAILURE);
+ }
+
+ timeout_end();
+}
+
+/* Return the next line from the control socket (without the trailing newline).
+ *
+ * The program terminates if a timeout occurs.
+ *
+ * The caller must free() the returned string.
+ */
+char *control_readln(void)
+{
+ char *buf = NULL;
+ size_t idx = 0;
+ size_t buflen = 0;
+
+ timeout_begin(TIMEOUT);
+
+ for (;;) {
+ ssize_t ret;
+
+ if (idx >= buflen) {
+ char *new_buf;
+
+ new_buf = realloc(buf, buflen + 80);
+ if (!new_buf) {
+ perror("realloc");
+ exit(EXIT_FAILURE);
+ }
+
+ buf = new_buf;
+ buflen += 80;
+ }
+
+ do {
+ ret = recv(control_fd, &buf[idx], 1, 0);
+ timeout_check("recv");
+ } while (ret < 0 && errno == EINTR);
+
+ if (ret == 0) {
+ fprintf(stderr, "unexpected EOF on control socket\n");
+ exit(EXIT_FAILURE);
+ }
+
+ if (ret != 1) {
+ perror("recv");
+ exit(EXIT_FAILURE);
+ }
+
+ if (buf[idx] == '\n') {
+ buf[idx] = '\0';
+ break;
+ }
+
+ idx++;
+ }
+
+ timeout_end();
+
+ return buf;
+}
+
+/* Wait until a given line is received or a timeout occurs */
+void control_expectln(const char *str)
+{
+ char *line;
+
+ line = control_readln();
+ if (strcmp(str, line) != 0) {
+ fprintf(stderr, "expected \"%s\" on control socket, got \"%s\"\n",
+ str, line);
+ exit(EXIT_FAILURE);
+ }
+
+ free(line);
+}
diff --git a/tools/testing/vsock/timeout.c b/tools/testing/vsock/timeout.c
new file mode 100644
index 000000000000..c49b3003b2db
--- /dev/null
+++ b/tools/testing/vsock/timeout.c
@@ -0,0 +1,64 @@
+/* Timeout API for single-threaded programs that use blocking
+ * syscalls (read/write/send/recv/connect/accept).
+ *
+ * Copyright (C) 2017 Red Hat, Inc.
+ *
+ * Author: Stefan Hajnoczi <stefanha@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+/* Use the following pattern:
+ *
+ * timeout_begin(TIMEOUT);
+ * do {
+ * ret = accept(...);
+ * timeout_check("accept");
+ * } while (ret < 0 && ret == EINTR);
+ * timeout_end();
+ */
+
+#include <stdlib.h>
+#include <stdbool.h>
+#include <unistd.h>
+#include <stdio.h>
+#include "timeout.h"
+
+static volatile bool timeout;
+
+/* SIGALRM handler function. Do not use sleep(2), alarm(2), or
+ * setitimer(2) while using this API - they may interfere with each
+ * other.
+ */
+void sigalrm(int signo)
+{
+ timeout = true;
+}
+
+/* Start a timeout. Call timeout_check() to verify that the timeout hasn't
+ * expired. timeout_end() must be called to stop the timeout. Timeouts cannot
+ * be nested.
+ */
+void timeout_begin(unsigned int seconds)
+{
+ alarm(seconds);
+}
+
+/* Exit with an error message if the timeout has expired */
+void timeout_check(const char *operation)
+{
+ if (timeout) {
+ fprintf(stderr, "%s timed out\n", operation);
+ exit(EXIT_FAILURE);
+ }
+}
+
+/* Stop a timeout */
+void timeout_end(void)
+{
+ alarm(0);
+ timeout = false;
+}
diff --git a/tools/testing/vsock/vsock_diag_test.c b/tools/testing/vsock/vsock_diag_test.c
new file mode 100644
index 000000000000..e896a4af52f4
--- /dev/null
+++ b/tools/testing/vsock/vsock_diag_test.c
@@ -0,0 +1,681 @@
+/*
+ * vsock_diag_test - vsock_diag.ko test suite
+ *
+ * Copyright (C) 2017 Red Hat, Inc.
+ *
+ * Author: Stefan Hajnoczi <stefanha@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <getopt.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <signal.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <linux/list.h>
+#include <linux/net.h>
+#include <linux/netlink.h>
+#include <linux/sock_diag.h>
+#include <netinet/tcp.h>
+
+#include "../../../include/uapi/linux/vm_sockets.h"
+#include "../../../include/uapi/linux/vm_sockets_diag.h"
+
+#include "timeout.h"
+#include "control.h"
+
+enum test_mode {
+ TEST_MODE_UNSET,
+ TEST_MODE_CLIENT,
+ TEST_MODE_SERVER
+};
+
+/* Per-socket status */
+struct vsock_stat {
+ struct list_head list;
+ struct vsock_diag_msg msg;
+};
+
+static const char *sock_type_str(int type)
+{
+ switch (type) {
+ case SOCK_DGRAM:
+ return "DGRAM";
+ case SOCK_STREAM:
+ return "STREAM";
+ default:
+ return "INVALID TYPE";
+ }
+}
+
+static const char *sock_state_str(int state)
+{
+ switch (state) {
+ case TCP_CLOSE:
+ return "UNCONNECTED";
+ case TCP_SYN_SENT:
+ return "CONNECTING";
+ case TCP_ESTABLISHED:
+ return "CONNECTED";
+ case TCP_CLOSING:
+ return "DISCONNECTING";
+ case TCP_LISTEN:
+ return "LISTEN";
+ default:
+ return "INVALID STATE";
+ }
+}
+
+static const char *sock_shutdown_str(int shutdown)
+{
+ switch (shutdown) {
+ case 1:
+ return "RCV_SHUTDOWN";
+ case 2:
+ return "SEND_SHUTDOWN";
+ case 3:
+ return "RCV_SHUTDOWN | SEND_SHUTDOWN";
+ default:
+ return "0";
+ }
+}
+
+static void print_vsock_addr(FILE *fp, unsigned int cid, unsigned int port)
+{
+ if (cid == VMADDR_CID_ANY)
+ fprintf(fp, "*:");
+ else
+ fprintf(fp, "%u:", cid);
+
+ if (port == VMADDR_PORT_ANY)
+ fprintf(fp, "*");
+ else
+ fprintf(fp, "%u", port);
+}
+
+static void print_vsock_stat(FILE *fp, struct vsock_stat *st)
+{
+ print_vsock_addr(fp, st->msg.vdiag_src_cid, st->msg.vdiag_src_port);
+ fprintf(fp, " ");
+ print_vsock_addr(fp, st->msg.vdiag_dst_cid, st->msg.vdiag_dst_port);
+ fprintf(fp, " %s %s %s %u\n",
+ sock_type_str(st->msg.vdiag_type),
+ sock_state_str(st->msg.vdiag_state),
+ sock_shutdown_str(st->msg.vdiag_shutdown),
+ st->msg.vdiag_ino);
+}
+
+static void print_vsock_stats(FILE *fp, struct list_head *head)
+{
+ struct vsock_stat *st;
+
+ list_for_each_entry(st, head, list)
+ print_vsock_stat(fp, st);
+}
+
+static struct vsock_stat *find_vsock_stat(struct list_head *head, int fd)
+{
+ struct vsock_stat *st;
+ struct stat stat;
+
+ if (fstat(fd, &stat) < 0) {
+ perror("fstat");
+ exit(EXIT_FAILURE);
+ }
+
+ list_for_each_entry(st, head, list)
+ if (st->msg.vdiag_ino == stat.st_ino)
+ return st;
+
+ fprintf(stderr, "cannot find fd %d\n", fd);
+ exit(EXIT_FAILURE);
+}
+
+static void check_no_sockets(struct list_head *head)
+{
+ if (!list_empty(head)) {
+ fprintf(stderr, "expected no sockets\n");
+ print_vsock_stats(stderr, head);
+ exit(1);
+ }
+}
+
+static void check_num_sockets(struct list_head *head, int expected)
+{
+ struct list_head *node;
+ int n = 0;
+
+ list_for_each(node, head)
+ n++;
+
+ if (n != expected) {
+ fprintf(stderr, "expected %d sockets, found %d\n",
+ expected, n);
+ print_vsock_stats(stderr, head);
+ exit(EXIT_FAILURE);
+ }
+}
+
+static void check_socket_state(struct vsock_stat *st, __u8 state)
+{
+ if (st->msg.vdiag_state != state) {
+ fprintf(stderr, "expected socket state %#x, got %#x\n",
+ state, st->msg.vdiag_state);
+ exit(EXIT_FAILURE);
+ }
+}
+
+static void send_req(int fd)
+{
+ struct sockaddr_nl nladdr = {
+ .nl_family = AF_NETLINK,
+ };
+ struct {
+ struct nlmsghdr nlh;
+ struct vsock_diag_req vreq;
+ } req = {
+ .nlh = {
+ .nlmsg_len = sizeof(req),
+ .nlmsg_type = SOCK_DIAG_BY_FAMILY,
+ .nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP,
+ },
+ .vreq = {
+ .sdiag_family = AF_VSOCK,
+ .vdiag_states = ~(__u32)0,
+ },
+ };
+ struct iovec iov = {
+ .iov_base = &req,
+ .iov_len = sizeof(req),
+ };
+ struct msghdr msg = {
+ .msg_name = &nladdr,
+ .msg_namelen = sizeof(nladdr),
+ .msg_iov = &iov,
+ .msg_iovlen = 1,
+ };
+
+ for (;;) {
+ if (sendmsg(fd, &msg, 0) < 0) {
+ if (errno == EINTR)
+ continue;
+
+ perror("sendmsg");
+ exit(EXIT_FAILURE);
+ }
+
+ return;
+ }
+}
+
+static ssize_t recv_resp(int fd, void *buf, size_t len)
+{
+ struct sockaddr_nl nladdr = {
+ .nl_family = AF_NETLINK,
+ };
+ struct iovec iov = {
+ .iov_base = buf,
+ .iov_len = len,
+ };
+ struct msghdr msg = {
+ .msg_name = &nladdr,
+ .msg_namelen = sizeof(nladdr),
+ .msg_iov = &iov,
+ .msg_iovlen = 1,
+ };
+ ssize_t ret;
+
+ do {
+ ret = recvmsg(fd, &msg, 0);
+ } while (ret < 0 && errno == EINTR);
+
+ if (ret < 0) {
+ perror("recvmsg");
+ exit(EXIT_FAILURE);
+ }
+
+ return ret;
+}
+
+static void add_vsock_stat(struct list_head *sockets,
+ const struct vsock_diag_msg *resp)
+{
+ struct vsock_stat *st;
+
+ st = malloc(sizeof(*st));
+ if (!st) {
+ perror("malloc");
+ exit(EXIT_FAILURE);
+ }
+
+ st->msg = *resp;
+ list_add_tail(&st->list, sockets);
+}
+
+/*
+ * Read vsock stats into a list.
+ */
+static void read_vsock_stat(struct list_head *sockets)
+{
+ long buf[8192 / sizeof(long)];
+ int fd;
+
+ fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_SOCK_DIAG);
+ if (fd < 0) {
+ perror("socket");
+ exit(EXIT_FAILURE);
+ }
+
+ send_req(fd);
+
+ for (;;) {
+ const struct nlmsghdr *h;
+ ssize_t ret;
+
+ ret = recv_resp(fd, buf, sizeof(buf));
+ if (ret == 0)
+ goto done;
+ if (ret < sizeof(*h)) {
+ fprintf(stderr, "short read of %zd bytes\n", ret);
+ exit(EXIT_FAILURE);
+ }
+
+ h = (struct nlmsghdr *)buf;
+
+ while (NLMSG_OK(h, ret)) {
+ if (h->nlmsg_type == NLMSG_DONE)
+ goto done;
+
+ if (h->nlmsg_type == NLMSG_ERROR) {
+ const struct nlmsgerr *err = NLMSG_DATA(h);
+
+ if (h->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
+ fprintf(stderr, "NLMSG_ERROR\n");
+ else {
+ errno = -err->error;
+ perror("NLMSG_ERROR");
+ }
+
+ exit(EXIT_FAILURE);
+ }
+
+ if (h->nlmsg_type != SOCK_DIAG_BY_FAMILY) {
+ fprintf(stderr, "unexpected nlmsg_type %#x\n",
+ h->nlmsg_type);
+ exit(EXIT_FAILURE);
+ }
+ if (h->nlmsg_len <
+ NLMSG_LENGTH(sizeof(struct vsock_diag_msg))) {
+ fprintf(stderr, "short vsock_diag_msg\n");
+ exit(EXIT_FAILURE);
+ }
+
+ add_vsock_stat(sockets, NLMSG_DATA(h));
+
+ h = NLMSG_NEXT(h, ret);
+ }
+ }
+
+done:
+ close(fd);
+}
+
+static void free_sock_stat(struct list_head *sockets)
+{
+ struct vsock_stat *st;
+ struct vsock_stat *next;
+
+ list_for_each_entry_safe(st, next, sockets, list)
+ free(st);
+}
+
+static void test_no_sockets(unsigned int peer_cid)
+{
+ LIST_HEAD(sockets);
+
+ read_vsock_stat(&sockets);
+
+ check_no_sockets(&sockets);
+
+ free_sock_stat(&sockets);
+}
+
+static void test_listen_socket_server(unsigned int peer_cid)
+{
+ union {
+ struct sockaddr sa;
+ struct sockaddr_vm svm;
+ } addr = {
+ .svm = {
+ .svm_family = AF_VSOCK,
+ .svm_port = 1234,
+ .svm_cid = VMADDR_CID_ANY,
+ },
+ };
+ LIST_HEAD(sockets);
+ struct vsock_stat *st;
+ int fd;
+
+ fd = socket(AF_VSOCK, SOCK_STREAM, 0);
+
+ if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
+ perror("bind");
+ exit(EXIT_FAILURE);
+ }
+
+ if (listen(fd, 1) < 0) {
+ perror("listen");
+ exit(EXIT_FAILURE);
+ }
+
+ read_vsock_stat(&sockets);
+
+ check_num_sockets(&sockets, 1);
+ st = find_vsock_stat(&sockets, fd);
+ check_socket_state(st, TCP_LISTEN);
+
+ close(fd);
+ free_sock_stat(&sockets);
+}
+
+static void test_connect_client(unsigned int peer_cid)
+{
+ union {
+ struct sockaddr sa;
+ struct sockaddr_vm svm;
+ } addr = {
+ .svm = {
+ .svm_family = AF_VSOCK,
+ .svm_port = 1234,
+ .svm_cid = peer_cid,
+ },
+ };
+ int fd;
+ int ret;
+ LIST_HEAD(sockets);
+ struct vsock_stat *st;
+
+ control_expectln("LISTENING");
+
+ fd = socket(AF_VSOCK, SOCK_STREAM, 0);
+
+ timeout_begin(TIMEOUT);
+ do {
+ ret = connect(fd, &addr.sa, sizeof(addr.svm));
+ timeout_check("connect");
+ } while (ret < 0 && errno == EINTR);
+ timeout_end();
+
+ if (ret < 0) {
+ perror("connect");
+ exit(EXIT_FAILURE);
+ }
+
+ read_vsock_stat(&sockets);
+
+ check_num_sockets(&sockets, 1);
+ st = find_vsock_stat(&sockets, fd);
+ check_socket_state(st, TCP_ESTABLISHED);
+
+ control_expectln("DONE");
+ control_writeln("DONE");
+
+ close(fd);
+ free_sock_stat(&sockets);
+}
+
+static void test_connect_server(unsigned int peer_cid)
+{
+ union {
+ struct sockaddr sa;
+ struct sockaddr_vm svm;
+ } addr = {
+ .svm = {
+ .svm_family = AF_VSOCK,
+ .svm_port = 1234,
+ .svm_cid = VMADDR_CID_ANY,
+ },
+ };
+ union {
+ struct sockaddr sa;
+ struct sockaddr_vm svm;
+ } clientaddr;
+ socklen_t clientaddr_len = sizeof(clientaddr.svm);
+ LIST_HEAD(sockets);
+ struct vsock_stat *st;
+ int fd;
+ int client_fd;
+
+ fd = socket(AF_VSOCK, SOCK_STREAM, 0);
+
+ if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
+ perror("bind");
+ exit(EXIT_FAILURE);
+ }
+
+ if (listen(fd, 1) < 0) {
+ perror("listen");
+ exit(EXIT_FAILURE);
+ }
+
+ control_writeln("LISTENING");
+
+ timeout_begin(TIMEOUT);
+ do {
+ client_fd = accept(fd, &clientaddr.sa, &clientaddr_len);
+ timeout_check("accept");
+ } while (client_fd < 0 && errno == EINTR);
+ timeout_end();
+
+ if (client_fd < 0) {
+ perror("accept");
+ exit(EXIT_FAILURE);
+ }
+ if (clientaddr.sa.sa_family != AF_VSOCK) {
+ fprintf(stderr, "expected AF_VSOCK from accept(2), got %d\n",
+ clientaddr.sa.sa_family);
+ exit(EXIT_FAILURE);
+ }
+ if (clientaddr.svm.svm_cid != peer_cid) {
+ fprintf(stderr, "expected peer CID %u from accept(2), got %u\n",
+ peer_cid, clientaddr.svm.svm_cid);
+ exit(EXIT_FAILURE);
+ }
+
+ read_vsock_stat(&sockets);
+
+ check_num_sockets(&sockets, 2);
+ find_vsock_stat(&sockets, fd);
+ st = find_vsock_stat(&sockets, client_fd);
+ check_socket_state(st, TCP_ESTABLISHED);
+
+ control_writeln("DONE");
+ control_expectln("DONE");
+
+ close(client_fd);
+ close(fd);
+ free_sock_stat(&sockets);
+}
+
+static struct {
+ const char *name;
+ void (*run_client)(unsigned int peer_cid);
+ void (*run_server)(unsigned int peer_cid);
+} test_cases[] = {
+ {
+ .name = "No sockets",
+ .run_server = test_no_sockets,
+ },
+ {
+ .name = "Listen socket",
+ .run_server = test_listen_socket_server,
+ },
+ {
+ .name = "Connect",
+ .run_client = test_connect_client,
+ .run_server = test_connect_server,
+ },
+ {},
+};
+
+static void init_signals(void)
+{
+ struct sigaction act = {
+ .sa_handler = sigalrm,
+ };
+
+ sigaction(SIGALRM, &act, NULL);
+ signal(SIGPIPE, SIG_IGN);
+}
+
+static unsigned int parse_cid(const char *str)
+{
+ char *endptr = NULL;
+ unsigned long int n;
+
+ errno = 0;
+ n = strtoul(str, &endptr, 10);
+ if (errno || *endptr != '\0') {
+ fprintf(stderr, "malformed CID \"%s\"\n", str);
+ exit(EXIT_FAILURE);
+ }
+ return n;
+}
+
+static const char optstring[] = "";
+static const struct option longopts[] = {
+ {
+ .name = "control-host",
+ .has_arg = required_argument,
+ .val = 'H',
+ },
+ {
+ .name = "control-port",
+ .has_arg = required_argument,
+ .val = 'P',
+ },
+ {
+ .name = "mode",
+ .has_arg = required_argument,
+ .val = 'm',
+ },
+ {
+ .name = "peer-cid",
+ .has_arg = required_argument,
+ .val = 'p',
+ },
+ {
+ .name = "help",
+ .has_arg = no_argument,
+ .val = '?',
+ },
+ {},
+};
+
+static void usage(void)
+{
+ fprintf(stderr, "Usage: vsock_diag_test [--help] [--control-host=<host>] --control-port=<port> --mode=client|server --peer-cid=<cid>\n"
+ "\n"
+ " Server: vsock_diag_test --control-port=1234 --mode=server --peer-cid=3\n"
+ " Client: vsock_diag_test --control-host=192.168.0.1 --control-port=1234 --mode=client --peer-cid=2\n"
+ "\n"
+ "Run vsock_diag.ko tests. Must be launched in both\n"
+ "guest and host. One side must use --mode=client and\n"
+ "the other side must use --mode=server.\n"
+ "\n"
+ "A TCP control socket connection is used to coordinate tests\n"
+ "between the client and the server. The server requires a\n"
+ "listen address and the client requires an address to\n"
+ "connect to.\n"
+ "\n"
+ "The CID of the other side must be given with --peer-cid=<cid>.\n");
+ exit(EXIT_FAILURE);
+}
+
+int main(int argc, char **argv)
+{
+ const char *control_host = NULL;
+ const char *control_port = NULL;
+ int mode = TEST_MODE_UNSET;
+ unsigned int peer_cid = VMADDR_CID_ANY;
+ int i;
+
+ init_signals();
+
+ for (;;) {
+ int opt = getopt_long(argc, argv, optstring, longopts, NULL);
+
+ if (opt == -1)
+ break;
+
+ switch (opt) {
+ case 'H':
+ control_host = optarg;
+ break;
+ case 'm':
+ if (strcmp(optarg, "client") == 0)
+ mode = TEST_MODE_CLIENT;
+ else if (strcmp(optarg, "server") == 0)
+ mode = TEST_MODE_SERVER;
+ else {
+ fprintf(stderr, "--mode must be \"client\" or \"server\"\n");
+ return EXIT_FAILURE;
+ }
+ break;
+ case 'p':
+ peer_cid = parse_cid(optarg);
+ break;
+ case 'P':
+ control_port = optarg;
+ break;
+ case '?':
+ default:
+ usage();
+ }
+ }
+
+ if (!control_port)
+ usage();
+ if (mode == TEST_MODE_UNSET)
+ usage();
+ if (peer_cid == VMADDR_CID_ANY)
+ usage();
+
+ if (!control_host) {
+ if (mode != TEST_MODE_SERVER)
+ usage();
+ control_host = "0.0.0.0";
+ }
+
+ control_init(control_host, control_port, mode == TEST_MODE_SERVER);
+
+ for (i = 0; test_cases[i].name; i++) {
+ void (*run)(unsigned int peer_cid);
+
+ printf("%s...", test_cases[i].name);
+ fflush(stdout);
+
+ if (mode == TEST_MODE_CLIENT)
+ run = test_cases[i].run_client;
+ else
+ run = test_cases[i].run_server;
+
+ if (run)
+ run(peer_cid);
+
+ printf("ok\n");
+ }
+
+ control_cleanup();
+ return EXIT_SUCCESS;
+}
diff --git a/tools/testing/vsock/.gitignore b/tools/testing/vsock/.gitignore
new file mode 100644
index 000000000000..dc5f11faf530
--- /dev/null
+++ b/tools/testing/vsock/.gitignore
@@ -0,0 +1,2 @@
+*.d
+vsock_diag_test
diff --git a/tools/testing/vsock/README b/tools/testing/vsock/README
new file mode 100644
index 000000000000..2cc6d7302db6
--- /dev/null
+++ b/tools/testing/vsock/README
@@ -0,0 +1,36 @@
+AF_VSOCK test suite
+-------------------
+These tests exercise net/vmw_vsock/ host<->guest sockets for VMware, KVM, and
+Hyper-V.
+
+The following tests are available:
+
+ * vsock_diag_test - vsock_diag.ko module for listing open sockets
+
+The following prerequisite steps are not automated and must be performed prior
+to running tests:
+
+1. Build the kernel and these tests.
+2. Install the kernel and tests on the host.
+3. Install the kernel and tests inside the guest.
+4. Boot the guest and ensure that the AF_VSOCK transport is enabled.
+
+Invoke test binaries in both directions as follows:
+
+ # host=server, guest=client
+ (host)# $TEST_BINARY --mode=server \
+ --control-port=1234 \
+ --peer-cid=3
+ (guest)# $TEST_BINARY --mode=client \
+ --control-host=$HOST_IP \
+ --control-port=1234 \
+ --peer-cid=2
+
+ # host=client, guest=server
+ (guest)# $TEST_BINARY --mode=server \
+ --control-port=1234 \
+ --peer-cid=2
+ (host)# $TEST_BINARY --mode=client \
+ --control-port=$GUEST_IP \
+ --control-port=1234 \
+ --peer-cid=3
--
2.13.6
^ permalink raw reply related
* Re: [PATCH 1/3] wireless: iwlwifi: use bool instead of int
From: Luciano Coelho @ 2017-10-04 16:39 UTC (permalink / raw)
To: Joe Perches, Christoph Böhmwalder, johannes.berg,
emmanuel.grumbach, kvalo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <1507134405.4434.10.camel@perches.com>
On Wed, 2017-10-04 at 09:26 -0700, Joe Perches wrote:
> On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> > Change a usage of int in a boolean context to use the bool type
> > instead, as it
> > makes the intent of the function clearer and helps clarify its
> > semantics.
> >
> > Also eliminate the if/else and just return the boolean result
> > directly,
> > making the code more readable.
> >
> > Signed-off-by: Christoph Böhmwalder <christoph@boehmwalder.at>
> > ---
> > drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 12 +++++-------
> > 1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > index b7cd813ba70f..0eb815ae97e8 100644
> > --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > @@ -267,14 +267,12 @@ int iwl_phy_db_set_section(struct iwl_phy_db
> > *phy_db,
> > }
> > IWL_EXPORT_SYMBOL(iwl_phy_db_set_section);
> >
> > -static int is_valid_channel(u16 ch_id)
> > +static bool is_valid_channel(u16 ch_id)
> > {
> > - if (ch_id <= 14 ||
> > - (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> > - (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> > - (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1))
> > - return 1;
> > - return 0;
> > + return (ch_id <= 14 ||
> > + (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> > + (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> > + (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1));
> > }
>
> This might be more intelligble as separate tests
>
> static bool is_valid_channel(u16 ch_id)
> {
> if (ch_id <= 14)
> return true;
>
> if ((ch_id % 4 == 0) &&
> ((ch_id >= 36 && ch_id <= 64) ||
> (ch_id >= 100 && ch_id <= 140)))
> return true;
>
> if ((ch_id % 4 == 1) &&
> (chid >= 145 && ch_id <= 165))
> return true;
>
> return false;
> }
>
> The compiler should produce the same object code.
Yeah, it may be a bit easier to read, but I don't want to start getting
"fixes" to working and reasonable code. There's nothing wrong with the
existing function (except maybe for the int vs. boolean) so let's not
change it.
A good time to change this would be the next time someone adds yet
another range of valid channels here. ;)
--
Luca.
^ permalink raw reply
* Re: etsec2 attached to sgmii phy
From: Jörg Willmann @ 2017-10-04 16:40 UTC (permalink / raw)
To: Andrew Lunn, netdev; +Cc: timur
In-Reply-To: <20171004153422.GC3435@lunn.ch>
Am 04.10.2017 um 17:34 schrieb Andrew Lunn:
> On Wed, Oct 04, 2017 at 04:19:23PM +0200, Jörg Willmann wrote:
>> On Wed, 4 Oct 2017, Andrew Lunn wrote:
>>
>>> On Wed, Oct 04, 2017 at 07:56:53AM +0200, Jörg Willmann wrote:
>>>> Hi,
>>>>
>>>> we use a QorIQ P1011 connected via SGMII to a switch (Marvell 88E6352).
>>>> Currently we still use a really old linux kernel (2.6.33) successfully.
>>>>
>>>> For configuration of the MDIO Bus attached to the corresponding eTSEC/TBI
>>>> Phy we use the following settings in the device tree:
>>>>
>>>> mdio@25000 {
>>>> #address-cells = <0x1>;
>>>> #size-cells = <0x0>;
>>>> compatible = "fsl,etsec2-tbi";
>>>> reg = <0x25000 0x1000 0xb1030 0x4>;
>>> Hi Joerg
>>>
>>> Is 0xb1030 0x4 fixed by the silicon? Can it be expressed as an offset from
>>> 0x25000?
>>>
>>> It seems like the idea behind the patch is to hard code some
>>> things. If you can hard code the offset into get_etsec_tbipa(), i
>>> think that would be an O.K. solution to your problem.
>>>
>>> Andrew
>>>
>> Yes, the adress 0xb1030 is fixed but it's something totally different than
>> the address range of 0x25000. 0xb0000, 0xb1000 and 0xb2000 are base
>> addresses of the eTSEC MAC (TPIPA is a register within the MAC) and 0x24000,
>> 0x25000 and 0x26000 are the base registers of the corresponding MDIO
>> controllers. So I wouldn't add a dependency between these two things.
>> >From my point of view, the implementation in the old kernel where
>> get_gfar_tbipa() got the device tree node pointer as argument was not soo
>> bad ;-)
> I took a quick look at the current device tree files. They all seem to
> have the 0xb1030 0x4. So reading it inside of get_etsec_tbipa() is
> O.K.
>
> Looks like you need to modify all the get_tbipa() functions to take a
> device_node *, and this code looks like it needs to change:
>
> /*
> * Add consistency check to make sure TBI is contained
> * within the mapped range (not because we would get a
> * segfault, rather to catch bugs in computing TBI
> * address). Print error message but continue anyway.
> */
> if ((void *)tbipa > priv->map + resource_size(&res) - 4)
> dev_err(&pdev->dev, "invalid register map (should be at least 0x%04zx to contain TBI address)\n",
> ((void *)tbipa - priv->map) + 4);
>
> iowrite32be(be32_to_cpup(prop), tbipa);
>
> Andrew
>
Yes, exactly - I already stumbled over these lines, too. Are there any
suggestions how to implement this the best way?
^ permalink raw reply
* Re: [PATCH v4 2/2] ip_tunnel: add mpls over gre encapsulation
From: Amine Kherbouche @ 2017-10-04 17:03 UTC (permalink / raw)
To: Tom Herbert; +Cc: Linux Kernel Network Developers, xeb, roopa, equinox
In-Reply-To: <CALx6S35b6gk39KOXTTzCFiz4S+JOOxcdJGB-+n3nx0x=xxVi8Q@mail.gmail.com>
On 09/29/2017 06:11 AM, Tom Herbert wrote:
>> > @@ -122,6 +125,30 @@ int gre_parse_header(struct sk_buff *skb, struct tnl_ptk_info *tpi,
>> > }
>> > EXPORT_SYMBOL(gre_parse_header);
>> >
>> > +#if IS_ENABLED(CONFIG_MPLS)
>> > +int mpls_gre_rcv(struct sk_buff *skb, int gre_hdr_len)
>> > +{
>> > + if (unlikely(!pskb_may_pull(skb, gre_hdr_len)))
>> > + goto drop;
>> > +
>> > + /* Pop GRE hdr and reset the skb */
>> > + skb_pull(skb, gre_hdr_len);
>> > + skb_reset_network_header(skb);
>> > +
> I don't see why MPLS/GRE needs to be a special case in gre_rcv. Can't
> we just follow the normal processing patch which calls the proto ops
> handler for the protocol in the GRE header? Also, if protocol specific
> code is added to rcv function that most likely means that we need to
> update the related offloads also (grant it that MPLS doesn't support
> GRO but it looks like it supports GSO). Additionally, we'd need to
> consider if flow dissector needs a similar special case (I will point
> out that my recently posted patches there eliminated TEB as the one
> special case in GRE dissection).
>
> Thanks,
> Tom
Hi Tom,
Thanks for the feedback, I think this is the best way to do it, I'll do
a v6 asap.
Regards,
Amine
^ permalink raw reply
* [PATCH v2 net-next 1/2] tcp: uniform the set up of sockets after successful connection
From: Wei Wang @ 2017-10-04 17:03 UTC (permalink / raw)
To: David Miller, netdev; +Cc: Yuchung Cheng, Neal Cardwell, Eric Dumazet, Wei Wang
From: Wei Wang <weiwan@google.com>
Currently in the TCP code, the initialization sequence for cached
metrics, congestion control, BPF, etc, after successful connection
is very inconsistent. This introduces inconsistent bevhavior and is
prone to bugs. The current call sequence is as follows:
(1) for active case (tcp_finish_connect() case):
tcp_mtup_init(sk);
icsk->icsk_af_ops->rebuild_header(sk);
tcp_init_metrics(sk);
tcp_call_bpf(sk, BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB);
tcp_init_congestion_control(sk);
tcp_init_buffer_space(sk);
(2) for passive case (tcp_rcv_state_process() TCP_SYN_RECV case):
icsk->icsk_af_ops->rebuild_header(sk);
tcp_call_bpf(sk, BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB);
tcp_init_congestion_control(sk);
tcp_mtup_init(sk);
tcp_init_buffer_space(sk);
tcp_init_metrics(sk);
(3) for TFO passive case (tcp_fastopen_create_child()):
inet_csk(child)->icsk_af_ops->rebuild_header(child);
tcp_init_congestion_control(child);
tcp_mtup_init(child);
tcp_init_metrics(child);
tcp_call_bpf(child, BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB);
tcp_init_buffer_space(child);
This commit uniforms the above functions to have the following sequence:
tcp_mtup_init(sk);
icsk->icsk_af_ops->rebuild_header(sk);
tcp_init_metrics(sk);
tcp_call_bpf(sk, BPF_SOCK_OPS_ACTIVE/PASSIVE_ESTABLISHED_CB);
tcp_init_congestion_control(sk);
tcp_init_buffer_space(sk);
This sequence is the same as the (1) active case. We pick this sequence
because this order correctly allows BPF to override the settings
including congestion control module and initial cwnd, etc from
the route, and then allows the CC module to see those settings.
Suggested-by: Neal Cardwell <ncardwell@google.com>
Tested-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
---
change in v2:
removed EXPORT_SYMBOL(tcp_init_transfer);
include/net/tcp.h | 1 +
net/ipv4/tcp.c | 12 ++++++++++++
net/ipv4/tcp_fastopen.c | 7 +------
net/ipv4/tcp_input.c | 21 +++------------------
4 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 770b608c8439..f45fdc57d29d 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -417,6 +417,7 @@ bool tcp_peer_is_proven(struct request_sock *req, struct dst_entry *dst);
void tcp_disable_fack(struct tcp_sock *tp);
void tcp_close(struct sock *sk, long timeout);
void tcp_init_sock(struct sock *sk);
+void tcp_init_transfer(struct sock *sk, int bpf_op);
unsigned int tcp_poll(struct file *file, struct socket *sock,
struct poll_table_struct *wait);
int tcp_getsockopt(struct sock *sk, int level, int optname,
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5091402720ab..3ed21e281c39 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -456,6 +456,18 @@ void tcp_init_sock(struct sock *sk)
}
EXPORT_SYMBOL(tcp_init_sock);
+void tcp_init_transfer(struct sock *sk, int bpf_op)
+{
+ struct inet_connection_sock *icsk = inet_csk(sk);
+
+ tcp_mtup_init(sk);
+ icsk->icsk_af_ops->rebuild_header(sk);
+ tcp_init_metrics(sk);
+ tcp_call_bpf(sk, bpf_op);
+ tcp_init_congestion_control(sk);
+ tcp_init_buffer_space(sk);
+}
+
static void tcp_tx_timestamp(struct sock *sk, u16 tsflags, struct sk_buff *skb)
{
if (tsflags && skb) {
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index e3c33220c418..515a757f02a8 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -216,12 +216,7 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
refcount_set(&req->rsk_refcnt, 2);
/* Now finish processing the fastopen child socket. */
- inet_csk(child)->icsk_af_ops->rebuild_header(child);
- tcp_init_congestion_control(child);
- tcp_mtup_init(child);
- tcp_init_metrics(child);
- tcp_call_bpf(child, BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB);
- tcp_init_buffer_space(child);
+ tcp_init_transfer(child, BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB);
tp->rcv_nxt = TCP_SKB_CB(skb)->seq + 1;
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index db9bb46b5776..bd3a35f5dbf2 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5513,20 +5513,13 @@ void tcp_finish_connect(struct sock *sk, struct sk_buff *skb)
security_inet_conn_established(sk, skb);
}
- /* Make sure socket is routed, for correct metrics. */
- icsk->icsk_af_ops->rebuild_header(sk);
-
- tcp_init_metrics(sk);
- tcp_call_bpf(sk, BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB);
- tcp_init_congestion_control(sk);
+ tcp_init_transfer(sk, BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB);
/* Prevent spurious tcp_cwnd_restart() on first data
* packet.
*/
tp->lsndtime = tcp_jiffies32;
- tcp_init_buffer_space(sk);
-
if (sock_flag(sk, SOCK_KEEPOPEN))
inet_csk_reset_keepalive_timer(sk, keepalive_time_when(tp));
@@ -5693,7 +5686,6 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
if (tcp_is_sack(tp) && sysctl_tcp_fack)
tcp_enable_fack(tp);
- tcp_mtup_init(sk);
tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
tcp_initialize_rcv_mss(sk);
@@ -5920,14 +5912,8 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
inet_csk(sk)->icsk_retransmits = 0;
reqsk_fastopen_remove(sk, req, false);
} else {
- /* Make sure socket is routed, for correct metrics. */
- icsk->icsk_af_ops->rebuild_header(sk);
- tcp_call_bpf(sk, BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB);
- tcp_init_congestion_control(sk);
-
- tcp_mtup_init(sk);
+ tcp_init_transfer(sk, BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB);
tp->copied_seq = tp->rcv_nxt;
- tcp_init_buffer_space(sk);
}
smp_mb();
tcp_set_state(sk, TCP_ESTABLISHED);
@@ -5957,8 +5943,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
* are sent out.
*/
tcp_rearm_rto(sk);
- } else
- tcp_init_metrics(sk);
+ }
if (!inet_csk(sk)->icsk_ca_ops->cong_control)
tcp_update_pacing_rate(sk);
--
2.14.2.920.gcf0c67979c-goog
^ permalink raw reply related
* [PATCH v2 net-next 2/2] tcp: clean up TFO server's initial tcp_rearm_rto() call
From: Wei Wang @ 2017-10-04 17:04 UTC (permalink / raw)
To: David Miller, netdev; +Cc: Yuchung Cheng, Neal Cardwell, Eric Dumazet, Wei Wang
From: Wei Wang <weiwan@google.com>
This commit does a cleanup and moves tcp_rearm_rto() call in the TFO
server case into a previous spot in tcp_rcv_state_process() to make
it more compact.
This is only a cosmetic change.
Suggested-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
---
no change in v2
net/ipv4/tcp_input.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index bd3a35f5dbf2..c5b8d61846c2 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5911,6 +5911,15 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
if (req) {
inet_csk(sk)->icsk_retransmits = 0;
reqsk_fastopen_remove(sk, req, false);
+ /* Re-arm the timer because data may have been sent out.
+ * This is similar to the regular data transmission case
+ * when new data has just been ack'ed.
+ *
+ * (TFO) - we could try to be more aggressive and
+ * retransmitting any data sooner based on when they
+ * are sent out.
+ */
+ tcp_rearm_rto(sk);
} else {
tcp_init_transfer(sk, BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB);
tp->copied_seq = tp->rcv_nxt;
@@ -5933,18 +5942,6 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
if (tp->rx_opt.tstamp_ok)
tp->advmss -= TCPOLEN_TSTAMP_ALIGNED;
- if (req) {
- /* Re-arm the timer because data may have been sent out.
- * This is similar to the regular data transmission case
- * when new data has just been ack'ed.
- *
- * (TFO) - we could try to be more aggressive and
- * retransmitting any data sooner based on when they
- * are sent out.
- */
- tcp_rearm_rto(sk);
- }
-
if (!inet_csk(sk)->icsk_ca_ops->cong_control)
tcp_update_pacing_rate(sk);
--
2.14.2.920.gcf0c67979c-goog
^ permalink raw reply related
* Re: [PATCH v2 net-next] ravb: RX checksum offload
From: David Miller @ 2017-10-04 17:26 UTC (permalink / raw)
To: horms+renesas; +Cc: sergei.shtylyov, magnus.damm, netdev, linux-renesas-soc
In-Reply-To: <1507103667-6084-1-git-send-email-horms+renesas@verge.net.au>
From: Simon Horman <horms+renesas@verge.net.au>
Date: Wed, 4 Oct 2017 09:54:27 +0200
> Add support for RX checksum offload. This is enabled by default and
> may be disabled and re-enabled using ethtool:
>
> # ethtool -K eth0 rx off
> # ethtool -K eth0 rx on
>
> The RAVB provides a simple checksumming scheme which appears to be
> completely compatible with CHECKSUM_COMPLETE: sum of all packet data after
> the L2 header is appended to packet data; this may be trivially read by the
> driver and used to update the skb accordingly.
>
> In terms of performance throughput is close to gigabit line-rate both with
> and without RX checksum offload enabled. Perf output, however, appears to
> indicate that significantly less time is spent in do_csum(). This is as
> expected.
...
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Applied, thanks Simon.
^ permalink raw reply
* Re: [PATCH net-next] net: core: fix kerneldoc comment
From: David Miller @ 2017-10-04 17:28 UTC (permalink / raw)
To: fw; +Cc: netdev
In-Reply-To: <20171004115650.6406-1-fw@strlen.de>
From: Florian Westphal <fw@strlen.de>
Date: Wed, 4 Oct 2017 13:56:50 +0200
> net/core/dev.c:1306: warning: No description found for parameter 'name'
> net/core/dev.c:1306: warning: Excess function parameter 'alias' description in 'dev_get_alias'
>
> Fixes: 6c5570016b97 ("net: core: decouple ifalias get/set from rtnl lock")
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Applied.
^ permalink raw reply
* Re: [PATCH] cxgb4vf: make a couple of functions static
From: David Miller @ 2017-10-04 17:32 UTC (permalink / raw)
To: colin.king; +Cc: leedom, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20171004132037.6409-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Wed, 4 Oct 2017 14:20:37 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> The functions t4vf_link_down_rc_str and t4vf_handle_get_port_info are
> local to the source and do not need to be in global scope, so make
> them static.
>
> Cleans up sparse warnings:
> symbol 't4vf_link_down_rc_str' was not declared. Should it be static?
> symbol 't4vf_handle_get_port_info' was not declared. Should it be static?
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH net-next] rtnetlink: remove slave_validate callback
From: David Miller @ 2017-10-04 17:34 UTC (permalink / raw)
To: fw; +Cc: netdev, jiri
In-Reply-To: <20171004135529.3967-1-fw@strlen.de>
From: Florian Westphal <fw@strlen.de>
Date: Wed, 4 Oct 2017 15:55:29 +0200
> no users in the tree.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] rtnetlink: remove __rtnl_af_unregister
From: David Miller @ 2017-10-04 17:34 UTC (permalink / raw)
To: fw; +Cc: netdev
In-Reply-To: <20171004135849.4368-1-fw@strlen.de>
From: Florian Westphal <fw@strlen.de>
Date: Wed, 4 Oct 2017 15:58:49 +0200
> switch the only caller to rtnl_af_unregister.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Applied.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox