Linux Documentation
 help / color / mirror / Atom feed
From: Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>
To: "Maciej Żenczykowski" <maze@google.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Cc: onathan Corbet <corbet@lwn.net>,
	Linyu Yuan <quic_linyyuan@quicinc.com>,
	<linux-usb@vger.kernel.org>, <linux-doc@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <quic_ppratap@quicinc.com>,
	<quic_wcheng@quicinc.com>, <quic_jackp@quicinc.com>
Subject: Re: [PATCH 2/2] usb: gadget: ncm: Add support to update wMaxSegmentSize via configfs
Date: Thu, 12 Oct 2023 21:10:46 +0530	[thread overview]
Message-ID: <8ff92053-52ff-4950-95c8-0e986f6a028a@quicinc.com> (raw)
In-Reply-To: <CANP3RGcqWBYd9FqAX47rE9pFgBTB8=0CGdwkScm-OH1epHcVWQ@mail.gmail.com>



On 10/12/2023 6:02 PM, Maciej Żenczykowski wrote:
> On Thu, Oct 12, 2023 at 1:48 AM Krishna Kurapati PSSNV
> 
> Could you paste the full patch?
> This is hard to review without looking at much more context then email
> is providing
> (or, even better, send me a link to a CL in gerrit somewhere - for
> example aosp ACK mainline tree)

Sure. Will provide a gerrit on ACK for review before posting v2.

The intent of posting the diff was two fold:

1. The question Greg asked regarding why the max segment size was 
limited to 15014 was valid. When I thought about it, I actually wanted 
to limit the max MTU to 15000, so the max segment size automatically 
needs to be limited to 15014. But my commit text didn't mention this 
properly which was a mistake on my behalf. But when I looked at the 
code, limiting the max segment size 15014 would force the practical 
max_mtu to not cross 15000 although theoretical max_mtu was set to:
(GETHER_MAX_MTU_SIZE - 15412) during registration of net device.

So my assumption of limiting it to 15000 was wrong. It must be limited 
to 15412 as mentioned in u_ether.c  This inturn means we must limit 
max_segment_size to:
GETHER_MAX_ETH_FRAME_LEN (GETHER_MAX_MTU_SIZE + ETH_HLEN)
as mentioned in u_ether.c.

I wanted to confirm that setting MAX_DATAGRAM_SIZE to 
GETHER_MAX_ETH_FRAME_LEN was correct.

2. I am not actually able to test with MTU beyond 15000. When my host 
device is a linux machine, the cdc_ncm.c limits max_segment_size to:
CDC_NCM_MAX_DATAGRAM_SIZE		8192	/* bytes */

When connected to windows machine, I am able to set the mtu to a max 
value of 15000. So not sure how to test the patch if I set the 
max_segment_size to GETHER_MAX_ETH_FRAME_LEN.

By pasting the diff, I wanted to confirm both the above queries.

And you are right, while assigning value to ecm.wMaxSegmentSize, we must 
use cpu_to_le16(...). Will ensure to make this change in v2. It worked 
without that too, not sure how.

Let me know your thoughts on the above.

Regards,
Krishna,

  reply	other threads:[~2023-10-12 15:41 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-09 14:20 [PATCH 1/2] Documentation: usb: Update NCM configfs parameters Krishna Kurapati
2023-10-09 14:20 ` [PATCH 2/2] usb: gadget: ncm: Add support to update wMaxSegmentSize via configfs Krishna Kurapati
2023-10-09 15:08   ` Greg Kroah-Hartman
2023-10-09 15:32     ` Krishna Kurapati PSSNV
2023-10-09 17:54       ` Greg Kroah-Hartman
2023-10-09 18:27         ` Krishna Kurapati PSSNV
2023-10-10  0:17   ` Maciej Żenczykowski
2023-10-10  0:20     ` Maciej Żenczykowski
2023-10-10  0:37       ` Maciej Żenczykowski
2023-10-10  4:38         ` Krishna Kurapati PSSNV
2023-10-12  8:48           ` Krishna Kurapati PSSNV
2023-10-12 12:32             ` Maciej Żenczykowski
2023-10-12 15:40               ` Krishna Kurapati PSSNV [this message]
2023-10-13 18:39                 ` Maciej Żenczykowski
2023-10-13 18:40                   ` Maciej Żenczykowski
2023-10-13 19:58                   ` Krishna Kurapati PSSNV
2023-10-13 22:35                     ` Maciej Żenczykowski
2023-10-14  7:02                       ` Krishna Kurapati PSSNV
2023-10-14  8:23                         ` Krishna Kurapati PSSNV
2023-10-16  1:19                           ` Maciej Żenczykowski
2023-10-16  3:48                             ` Krishna Kurapati PSSNV
2023-10-10  4:34       ` Krishna Kurapati PSSNV
2023-10-10  4:27     ` Krishna Kurapati PSSNV
2023-10-10 22:26   ` kernel test robot
2023-10-09 15:05 ` [PATCH 1/2] Documentation: usb: Update NCM configfs parameters Greg Kroah-Hartman
2023-10-09 15:10   ` Krishna Kurapati PSSNV
2023-10-09 15:21     ` Greg Kroah-Hartman
2023-10-09 15:33       ` Krishna Kurapati PSSNV
2023-10-09 15:42         ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8ff92053-52ff-4950-95c8-0e986f6a028a@quicinc.com \
    --to=quic_kriskura@quicinc.com \
    --cc=corbet@lwn.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=maze@google.com \
    --cc=quic_jackp@quicinc.com \
    --cc=quic_linyyuan@quicinc.com \
    --cc=quic_ppratap@quicinc.com \
    --cc=quic_wcheng@quicinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox