check to the initial checks in fr_rx. fh->ea2 == 1 means
the second address byte is the final address byte. We only support the
case where the address length is 2 bytes.
Cc: Krzysztof Halasa
Signed-off-by: Xie He
---
drivers/net/wan/hdlc_fr.c | 2 +-
1 file changed, 1 insertion(+), 1 d
devices) to upper layers. Because we don't use header_ops for normal
PVC devices, we should hide the header from upper layer code in this case.
Cc: Krzysztof Halasa
Signed-off-by: Xie He
---
drivers/net/wan/hdlc_fr.c | 76 ++-
1 file changed, 51 insertions(+
eated several times, this
patch uses "goto rx_drop" to replace them so that the code looks cleaner.
Also add code to increase the stats.rx_dropped count whenever we drop a
frame.
Cc: Krzysztof Halasa
Signed-off-by: Xie He
---
drivers/net/wan/hdlc_fr.c | 16 +++-
1 file change
eated several times, this
patch uses "goto rx_drop" to replace them so that the code looks cleaner.
Also add code to increase the stats.rx_dropped count whenever we drop a
frame.
Cc: Krzysztof Halasa
Signed-off-by: Xie He
---
drivers/net/wan/hdlc_fr.c | 16 +++-
1 file change
check to the initial checks in fr_fx. fh->ea2 == 1 means
the second address byte is the final address byte. We only support the
case where the address length is 2 bytes.
Cc: Krzysztof Halasa
Signed-off-by: Xie He
---
drivers/net/wan/hdlc_fr.c | 2 +-
1 file changed, 1 insertion(+), 1 d
: first make sure the pointer is not
NULL, then assign it directly to skb->dev. "dev" is no longer needed until
the end where we use it to update stats.
Cc: Krzysztof Halasa
Signed-off-by: Xie He
---
drivers/net/wan/hdlc_fr.c | 37 -
1 file chang
devices) to upper layers. Because we don't use header_ops for normal
PVC devices, we should hide the header from upper layer code in this case.
Cc: Krzysztof Halasa
Signed-off-by: Xie He
---
drivers/net/wan/hdlc_fr.c | 76 ++-
1 file changed, 51 insertions(+
On Tue, Oct 27, 2020 at 6:24 AM Arnd Bergmann wrote:
>
> Ah, of course. I had looked up the types but mixed up the memmap
> and HDW definitions, but then got confused trying to understand the
> logic in wr_mem() that operates on bytes but expands them into
> multiples of 4.
I think wr_mem() doesn
On Mon, Oct 26, 2020 at 8:56 PM Xie He wrote:
>
> > - for (mem = (HDW *) memmap; mem < (HDW *) (memmap + 1); ++mem)
> > + for (mem = (HDW *) memmap; mem < (HDW *) ((uintptr_t)memmap + 1); ++mem)
>
> Note that these two lines are semantically different. In the fi
> - for (mem = (HDW *) memmap; mem < (HDW *) (memmap + 1); ++mem)
> + for (mem = (HDW *) memmap; mem < (HDW *) ((uintptr_t)memmap + 1); ++mem)
Note that these two lines are semantically different. In the first line,
"+ 1" moves the pointer by (sizeof memmap) bytes. However in the second
line, "+
On Thu, Oct 22, 2020 at 6:56 PM Xie He wrote:
>
> My patch isn't complete. Because there are so many drivers with this
> problem, I feel it's hard to solve them all at once. So I only grepped
> "skb_padto" under "drivers/net/ethernet". There are other
On Thu, Oct 22, 2020 at 5:44 PM Jakub Kicinski wrote:
>
> On Thu, 22 Oct 2020 12:59:45 -0700 Xie He wrote:
> >
> > But I also see some drivers that want to pad the skb to a strange
> > length, and don't set their special min_mtu to match this length. For
> > ex
On Thu, Oct 22, 2020 at 8:22 AM Jakub Kicinski wrote:
>
> Are most of these drivers using skb_padto()? Is that the reason they
> can't be sharing the SKB?
Yes, I think if a driver calls skb_pad / skb_padto / skb_put_padto /
eth_skb_pad, the driver can't accept shared skbs because it may modify
th
ic function, it cannot be used anywhere else.
Fixes: 550fd08c2ceb ("net: Audit drivers to identify those needing
IFF_TX_SKB_SHARING cleared")
Cc: Neil Horman
Cc: Jakub Kicinski
Cc: David S. Miller
Signed-off-by: Xie He
---
drivers/net/ethernet/adaptec/starfire.c | 2 ++
drivers/
Sorry. I spotted some errors in this patch. Some drivers use "ndev" as
the variable name but I mistakenly used "dev".
It was very hard for me to attempt fixing. There are too many drivers
that need to be fixed. Fixing them is very time-consuming and may also
be error-prone. So I think it may be be
ic function, it cannot be used anywhere else.
Fixes: 550fd08c2ceb ("net: Audit drivers to identify those needing
IFF_TX_SKB_SHARING cleared")
Cc: Neil Horman
Cc: Jakub Kicinski
Cc: David S. Miller
Signed-off-by: Xie He
---
drivers/net/ethernet/adaptec/starfire.c | 2 ++
drivers/
On Wed, Oct 21, 2020 at 6:02 PM Jakub Kicinski wrote:
>
> Applied, thank you.
>
> In the future please try to provide a Fixes: tag.
OK. Thanks! I'll remember this in the future!
i_conf and frad_conf). According to the
comments, these two structs are specially crafted for sdla.c (the only
hardware driver dlci.c supports). I think this makes dlci.c not generic
enough to support other hardware drivers.
Cc: Lee Jones
Cc: Gustavo A. R. Silva
Cc: Krzysztof Kozlowski
Signed-o
transmission, so the skb may be modified.
Cc: Neil Horman
Cc: Krzysztof Halasa
Signed-off-by: Xie He
---
drivers/net/wan/hdlc_raw_eth.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wan/hdlc_raw_eth.c b/drivers/net/wan/hdlc_raw_eth.c
index 08e0a46501de..c70a518b8b47 100644
--- a
ore starting its
processing. This patch adds this check to hdlc_rcv.
Cc: Krzysztof Halasa
Signed-off-by: Xie He
---
drivers/net/wan/hdlc.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wan/hdlc.c b/drivers/net/wan/hdlc.c
index 9b00708676cf..1bdd3df08
On Mon, Oct 19, 2020 at 3:49 PM Jakub Kicinski wrote:
>
> Cool! FWIW when you resend you can also trim the subject to just say:
>
> net: hdlc: In hdlc_rcv, check to make sure dev is an HDLC device
>
> There's no need for the full file path.
OK. I'll do that. Thanks!
On Mon, Oct 19, 2020 at 2:22 PM Jakub Kicinski wrote:
>
> Looks correct to me. I spotted there is also IFF_WAN_HDLC added by
> 7cdc15f5f9db ("WAN: Generic HDLC now uses IFF_WAN_HDLC private flag.")
> would using that flag also be correct and cleaner potentially?
>
> Up to you, just wanted to make
_xmit to this).
Cc: Krzysztof Halasa
Signed-off-by: Xie He
---
drivers/net/wan/hdlc.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wan/hdlc.c b/drivers/net/wan/hdlc.c
index 9b00708676cf..0a392fb9aff8 100644
--- a/drivers/net/wan/hdlc.c
+++ b/drivers/ne
On Sun, Oct 18, 2020 at 3:05 PM Jakub Kicinski wrote:
>
> Whenever you make a list like that it's a strong indication that
> each of these should be a separate commit. That makes things easier
> to review.
>
>
> We have already sent a pull request for 5.10 and therefore net-next
> is closed for ne
nce when we actually need it.
Also add an fh->ea2 check to the initial checks in fr_fx. fh->ea2 == 1
means the second address byte is the final address byte. We only support
the case where the address length is 2 bytes.
4. Use "goto rx_drop" whenever w
pointers (skb_p) because we no
longer need to replace the skb inside fr_hard_header.
Cc: Krzysztof Halasa
Signed-off-by: Xie He
---
drivers/net/wan/hdlc_fr.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wan/hdlc_fr.c b/drivers
ord "check" in the comments because pskb_may_pull may
do things other than simple checks in the case of non-linear skbs.)
Cc: Willem de Bruijn
Cc: Martin Schiller
Signed-off-by: Xie He
---
drivers/net/wan/hdlc_x25.c | 4 ++--
drivers/net/wan/lapbether.c | 4 ++--
drivers/net/wan/x2
andling code.
"kfree_skb" is the correct function to call when dropping an skb due to
an error. "dev_kfree_skb", which is an alias of "consume_skb", is for
dropping skbs normally (not due to an error).
Cc: Krzysztof Halasa
Cc: Stephen
On Sat, Oct 3, 2020 at 11:10 AM Stephen Hemminger
wrote:
>
> This code snippet is basically an version of skb_pad().
> Probably it is very old and pre-dates that.
> Could the code use skb_pad?
Oh! Yes! I looked at the skb_pad function and I think we can use it in
this code.
Since it doesn't do s
b" in error handling code.
"kfree_skb" is the correct function to call when dropping an skb due to
an error. "dev_kfree_skb", which is an alias of "consume_skb", is for
dropping skbs normally (not due to an error).
Cc: Krzysztof Halasa
On Mon, Sep 28, 2020 at 3:58 PM David Miller wrote:
>
> It could also go back down and also back up again after you do this
> test. Maybe even 10 or 100 times over.
>
> You can't just leave things so incredibly racy like this, please apply
> proper synchronization between netdev state changes and
_ETHER; and for situation 3, skb->dev->type would be ARPHRD_DLCI.
This way fr_hard_header would be able to distinguish these 3 situations
correctly regardless what skb->protocol value the user tries to use in
situation 3.
Cc: Krzysztof Halasa
Signed-off-by: Xie He
locking to prevent this.
This needs to be fixed.)
Cc: Martin Schiller
Signed-off-by: Xie He
---
drivers/net/wan/x25_asy.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wan/x25_asy.c b/drivers/net/wan/x25_asy.c
index c418767a890a..22fcc0dd4b57 10
s ndo_stop, and
made it call the original ndo_stop function. I moved netif_stop_queue
from the original ndo_stop function to the new ndo_stop function.)
Cc: Martin Schiller
Signed-off-by: Xie He
---
Change from v1:
In v1, I moved the x25_asy_close call to x25_asy_close_tty.
Now, although I still
Sorry. I wish to drop the 3rd part (the 3rd point in the commit
message) and re-send this patch.
I'll re-address the problem discussed in the 3rd point in future patches.
because x25_asy_open is called in x25_asy_open_tty.)
Cc: Martin Schiller
Signed-off-by: Xie He
---
drivers/net/wan/x25_asy.c | 42 +++
1 file changed, 25 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wan/x25_asy.c b/drivers/net/wan/x25_asy.c
i
skb->nh.raw has been renamed as skb->network_header in 2007, in
commit b0e380b1d8a8 ("[SK_BUFF]: unions of just one member don't get
anything done, kill them")
So here we change it to the new name.
Cc: Willem de Bruijn
Signed-off-by: Xie He
---
net/p
On Thu, Sep 17, 2020 at 1:51 AM Willem de Bruijn
wrote:
>
> Acked-by: Willem de Bruijn
Thank you, Willem!
s).
Cc: Krzysztof Halasa
Signed-off-by: Xie He
---
drivers/net/wan/hdlc_cisco.c | 1 +
drivers/net/wan/hdlc_fr.c| 3 +++
drivers/net/wan/hdlc_ppp.c | 1 +
3 files changed, 5 insertions(+)
diff --git a/drivers/net/wan/hdlc_cisco.c b/drivers/net/wan/hdlc_cisco.c
index 444130655d8e..cb58
akes a
user listening on the Ethernet device with an AF_PACKET socket, to see
the same sll_protocol value for incoming and outgoing frames.
Cc: Martin Schiller
Signed-off-by: Xie He
---
drivers/net/wan/lapbether.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/n
y tries to restore
the LL header when header_ops != NULL.
Cc: Willem de Bruijn
Signed-off-by: Xie He
---
net/packet/af_packet.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2d5d5fbb435c..f59f
means this usage is already adopted by driver developers.
Cc: Willem de Bruijn
Cc: Eric Dumazet
Cc: Brian Norris
Cc: Cong Wang
Signed-off-by: Xie He
---
net/packet/af_packet.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packe
On Sun, Sep 13, 2020 at 10:32 PM Martin Schiller wrote:
>
> Acked-by: Martin Schiller
Thank you, Martin!
On Sun, Sep 13, 2020 at 10:26 PM Krzysztof Hałasa wrote:
>
> Xie He writes:
>
> > The HDLC device is not actually prepending any header when it is used
> > with this driver. When the PVC device has prepended its header and
> > handed over the skb to the HDLC device, th
On Sun, Sep 13, 2020 at 1:11 AM Willem de Bruijn
wrote:
>
> I am concerned about adding a WARN_ON_ONCE that we already expect to
> fire on some platforms.
>
> Probably better to add the documentation without the warning.
>
> I know I suggested the check before, sorry for the churn, but I hadn't
>
only called before netif_rx and not before lapb_data_received.
Cc: Martin Schiller
Signed-off-by: Xie He
---
drivers/net/wan/x25_asy.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wan/x25_asy.c b/drivers/net/wan/x25_asy.c
index 5a7cf8bf9d0d..ab56a5e6447a
On Fri, Sep 11, 2020 at 2:44 PM David Miller wrote:
>
> From: Xie He
> Date: Thu, 10 Sep 2020 23:35:03 -0700
>
> > The "SLF_OUTWAIT" flag defined in x25_asy.h is not actually used.
> > It is only cleared at one place in x25_asy.c but is never read or set.
>
On Fri, Sep 11, 2020 at 7:32 AM Willem de Bruijn
wrote:
>
> From a quick scan, a few device types that might trigger this
>
> net/atm/clip.c
> drivers/net/wan/hdlc_fr.c
> drivers/net/appletalk/ipddp.c
> drivers/net/ppp/ppp_generic.c
> drivers/net/net_failover.c
I have recently fixed this problem
On Fri, Sep 11, 2020 at 2:41 PM David Miller wrote:
>
> From: Xie He
> Date: Thu, 10 Sep 2020 23:07:20 -0700
>
> > This header file is not actually used in this file. Let's remove it.
>
> How did you test this assertion? As Jakub showed, the
> dlci_ioctl_set
The "SLF_OUTWAIT" flag defined in x25_asy.h is not actually used.
It is only cleared at one place in x25_asy.c but is never read or set.
So we can remove it.
Signed-off-by: Xie He
---
drivers/net/wan/x25_asy.c | 2 --
drivers/net/wan/x25_asy.h | 1 -
2 files changed, 3 deletions(-)
wan/sdla.c
Note that the "Frame Relay" module is different from and unrelated to the
"HDLC Frame Relay" module at:
drivers/net/wan/hdlc_fr.c
I think maybe we can deprecate the "Frame Relay" module because we already
have the (newer) "HDLC Frame Relay" module
The author tried to set the WiFi driver's hard_header_len to the Ethernet
header length, and request additional header space internally needed by
setting needed_headroom.
This means this usage is already adopted by driver developers.
Cc: Willem de Bruijn
Cc: Eric Dumazet
Cc: Brian Norris
Cc: Con
OK. I'll make the changes you suggested and resubmit the patch. Thanks!
I'll drop the change to netdevice.h and the check for
dev_hard_header's return value. If there's still a need for something
similar to these, we can do them in a separate patch.
tional header space internally needed by
setting needed_headroom. This means this usage is already adopted by
driver developers.
Cc: Willem de Bruijn
Cc: Eric Dumazet
Cc: Brian Norris
Cc: Cong Wang
Signed-off-by: Xie He
---
Change from v1:
Small change to the commit message.
---
include/linux/netde
tional header space internally needed by
setting needed_headroom. This means this usage is already adopted by
driver developers.
Cc: Willem de Bruijn
Cc: Eric Dumazet
Cc: Brian Norris
Cc: Cong Wang
Signed-off-by: Xie He
---
include/linux/netdevice.h | 4 ++--
net/packet/af_packet.c| 19 +
On Tue, Sep 8, 2020 at 4:53 AM Willem de Bruijn
wrote:
>
> On Tue, Sep 8, 2020 at 1:04 PM Xie He wrote:
> >
> > I was recently looking at some drivers, and I felt that if af_packet.c
> > could help me filter out the invalid RAW frames, I didn't need to
> > chec
On Tue, Sep 8, 2020 at 1:56 AM Willem de Bruijn
wrote:
>
> > > > /*
> > > > Assumptions:
> > > > - - if device has no dev->hard_header routine, it adds and removes ll
> > > > header
> > > > - inside itself. In this case ll header is invisible outside of
> > > > device,
> > > > - b
On Tue, Sep 8, 2020 at 1:41 AM Willem de Bruijn
wrote:
>
> The intent is to bypass such validation to be able to test device
> drivers. Note that removing that may cause someone's test to start
> failing.
>
> > So there's no point in
> > keeping the ability to test this, either.
>
> I don't disag
Thank you for your comment!
On Mon, Sep 7, 2020 at 2:41 AM Willem de Bruijn
wrote:
>
> On Sun, Sep 6, 2020 at 5:18 AM Xie He wrote:
> >
> > This comment is outdated and no longer reflects the actual implementation
> > of af_packet.c.
>
> If it was previously true,
On Mon, Sep 7, 2020 at 2:06 AM Willem de Bruijn
wrote:
>
> The CAP_SYS_RAWIO exception indeed was requested to be able to
> purposely test devices against bad inputs. The gmane link
> unfortunately no longer works, but this was the discussion thread:
> https://www.mail-archive.com/netdev@vger.kern
by
driver developers.
Cc: Willem de Bruijn
Cc: Eric Dumazet
Cc: Brian Norris
Cc: Cong Wang
Signed-off-by: Xie He
---
net/packet/af_packet.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2b33e97
On Sat, Sep 5, 2020 at 3:24 PM Xie He wrote:
>
> Hi Willem,
>
> I have a question about the function dev_validate_header used in
> af_packet.c. Can you help me? Thanks!
>
> I see when the length of the data is smaller than hard_header_len, and
> when the user is "
Hi Willem,
I have a question about the function dev_validate_header used in
af_packet.c. Can you help me? Thanks!
I see when the length of the data is smaller than hard_header_len, and
when the user is "capable" enough, the function will accept it and pad
it with 0s, without validating the header
On Fri, Sep 4, 2020 at 9:36 PM Jakub Kicinski wrote:
>
> Applied to net, thank you!
Thank you, Jakub!
On Fri, Sep 4, 2020 at 6:28 PM Xie He wrote:
>
> The HDLC device is not actually prepending any header when it is used
> with this driver. When the PVC device has prepended its header and
> handed over the skb to the HDLC device, the HDLC device just hands it
> over to the hard
Thank you for your email, Jakub!
On Fri, Sep 4, 2020 at 3:14 PM Jakub Kicinski wrote:
>
> Since this is a tunnel protocol on top of HDLC interfaces, and
> hdlc_setup_dev() sets dev->hard_header_len = 16; should we actually
> set the needed_headroom to 10 + 16 = 26? I'm not clear on where/if
> hdl
all cases.
Cc: Krzysztof Halasa
Cc: Martin Schiller
Signed-off-by: Xie He
---
Change from v1:
English language fix for the commit message.
Changed "Ethernet-emulated" to "Ethernet-emulating" because the device
is emulating an Ethernet device, rather than being emulated b
cases.
Cc: Krzysztof Halasa
Cc: Martin Schiller
Signed-off-by: Xie He
---
drivers/net/wan/hdlc_fr.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c
index 9acad651ea1f..12b35404cd8e 100644
--- a/drivers/net/wan/hdlc_fr.c
m and
does not call dev_hard_header, but checks if the user has provided a
header of length hard_header_len (in function dev_validate_header).
Cc: Krzysztof Halasa
Cc: Martin Schiller
Signed-off-by: Xie He
---
Change from v1:
Small fix for the English grammar in the commit message.
---
drive
m and
does not call dev_hard_header, but checks if the user has provided a
header of length hard_header_len (in function dev_validate_header).
Cc: Krzysztof Halasa
Cc: Martin Schiller
Signed-off-by: Xie He
---
drivers/net/wan/hdlc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
;t have header_ops, I think their
hard_header_len should be set to 0. So I think maybe it's better to
change the default value in hdlc.c to 0 and let them take the default
value of 0.
What do you think?
Thanks!
Xie He
dlc_header).
Cc: Martin Schiller
Signed-off-by: Xie He
---
drivers/net/wan/hdlc_cisco.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wan/hdlc_cisco.c b/drivers/net/wan/hdlc_cisco.c
index d8cba3625c18..444130655d8e 100644
--- a/drivers/net/wan/hdlc_cisco.c
+++ b/drivers/net/
the
Ethernet header, and at the beginning of the Ethernet payload):
Because when this driver receives an skb from the Ethernet device, the
network_header is also set at this place.
Cc: Martin Schiller
Signed-off-by: Xie He
---
drivers/net/wan/lapbether.c | 2 ++
1 file changed, 2 inserti
On Mon, Aug 24, 2020 at 4:09 PM David Miller wrote:
>
> Applied, thank you.
Thank you!
The underlying Ethernet device may request necessary tailroom to be
allocated by setting needed_tailroom. This driver should also set
needed_tailroom to request the tailroom needed by the underlying
Ethernet device to be allocated.
Cc: Willem de Bruijn
Cc: Martin Schiller
Signed-off-by: Xie He
Hi Martin Schiller,
Can you help review this patch? Thanks! It is a very simple patch that
adds the "needed_tailroom" setting for this driver.
Thank you!
Xie He
7;s needed_tailroom. So I
submitted this patch. I see you previously also did a change related
to needed_tailroom to pppoe. Can you help review this patch? Thank you
so much!
The patch is at:
http://patchwork.ozlabs.org/project/netdev/patch/20200808175251.582781-1-xie.he.0...@gmail.com/
Thanks!
Xie He
I took some time to look at the history of needed_tailroom. I found it
was added in this commit:
f5184d267c1a (net: Allow netdevices to specify needed head/tailroom)
The author tried to make use of needed_tailroom at various places in
the kernel by replacing the macro LL_RESERVED_SPACE with his ne
On Fri, Aug 14, 2020 at 8:41 PM David Miller wrote:
>
> Applied, thanks.
Thank you, David!
eader_len) to
the default values. We add needed_headroom here so that needed_headroom
will also be reset.
Cc: Willem de Bruijn
Cc: Martin Schiller
Cc: Andrew Hendry
Signed-off-by: Xie He
---
drivers/net/wan/hdlc.c | 1 +
drivers/net/wan/hdlc_x25.c | 17 -
2 files ch
On Tue, Aug 11, 2020 at 10:32 AM David Miller wrote:
>
> Applied, thank you.
Thank you!
On Tue, Aug 11, 2020 at 3:50 AM Willem de Bruijn
wrote:
>
> > I became interested in X.25 when I was trying different address
> > families that Linux supported. I tried AF_X25 sockets. And then I
> > tried to use the X.25 link layer directly through AF_PACKET. I believe
> > both AF_X25 sockets and
On Mon, Aug 10, 2020 at 12:21 AM Willem de Bruijn
wrote:
>
> Acked-by: Willem de Bruijn
Thank you so much!
> > 1) I hope to set needed_headroom properly for all three X.25 drivers
> > (lapbether, x25_asy, hdlc_x25) in the kernel. So that the upper layer
> > (net/x25) can be changed to use neede
On Mon, Aug 10, 2020 at 12:32 AM Willem de Bruijn
wrote:
>
> What happens when a tunnel device passes a packet to these devices?
> That will also not have allocated the extra tailroom. Does that cause
> a bug?
I looked at the code in net/ipv4/ip_tunnel.c. It indeed appeared to me
that it didn't t
On Sun, Aug 9, 2020 at 2:13 AM Willem de Bruijn
wrote:
>
> The patch is analogous to commit c7ca03c216ac
> ("drivers/net/wan/lapbether: Added needed_headroom and a skb->len
> check").
>
> Seems to make sense based on call stack
>
> x25_asy_xmit // skb_pull(skb, 1)
> lapb_data_req
On Sun, Aug 9, 2020 at 1:48 AM Willem de Bruijn
wrote:
>
> Does this solve an actual observed bug?
>
> In many ways lapbeth is similar to tunnel devices. This is not common.
Thank you for your comment!
This doesn't solve a bug observed by me. But I think this should be
necessary considering the
adroom
When this driver transmits data,
first this driver will remove a pseudo header of 1 byte,
then the lapb module will prepend the LAPB header of 2 or 3 bytes.
So the value of needed_headroom in this driver should be 3 - 1.
Cc: Willem de Bruijn
Cc: Martin Schiller
Signed-off-by:
The underlying Ethernet device may request necessary tailroom to be
allocated by setting needed_tailroom. This driver should also set
needed_tailroom to request the tailroom needed by the underlying
Ethernet device to be allocated.
Cc: Willem de Bruijn
Cc: Martin Schiller
Signed-off-by: Xie He
On Thu, Aug 6, 2020 at 12:47 AM Willem de Bruijn
wrote:
>
> Acked-by: Willem de Bruijn
>
> The in-band signal byte is required, but stripped by lapbeth_xmit.
> Subsequent code will prefix additional headers, including an Ethernet
> link layer. The extra space needs to be reserved, but not pulled
I'm sorry I forgot to include the "net" prefix again. I remembered
"PATCH" but not "net" this time. I'll try to remember both next time.
If requested I can resend the patch with the correct prefix. Sorry.
] ? packet_parse_headers.isra.0+0xd2/0x110
[ 168.399297] dev_queue_xmit+0x10/0x20
[ 168.399298] packet_sendmsg+0xbf0/0x19b0
..
Cc: Willem de Bruijn
Cc: Martin Schiller
Cc: Brian Norris
Signed-off-by: Xie He
---
drivers/net/wan/lapbether.c | 10 +-
1 file changed, 9 insertions(+),
On Tue, Aug 4, 2020 at 10:23 PM Martin Schiller wrote:
>
> > Adding skb_cow before these skb_push calls would indeed help
> > preventing kernel panics, but that might not be the essential issue
> > here, and it might also prevent us from discovering the real issue. (I
> > guess this is also the re
On Tue, Aug 4, 2020 at 5:43 AM Martin Schiller wrote:
>
> I'm not an expert in the field, but after reading the commit message and
> the previous comments, I'd say that makes sense.
Thanks!
> Shouldn't this kernel panic be intercepted by a skb_cow() before the
> skb_push() in lapbeth_data_transm
On Tue, Aug 4, 2020 at 3:07 AM Xie He wrote:
>
> Maybe we could contact . It seems to be the
> manager of VGER mail lists.
Oh. No. Majordomo seems to be a robot.
On Tue, Aug 4, 2020 at 12:06 AM Willem de Bruijn wrote:
>
> > BTW: The linux x25 mailing list does not seem to work anymore. I've been
> > on it for some time now, but haven't received a single email from it.
> > I've tried to contact owner-linux-...@vger.kernel.org, but only got an
> > "undeliver
On Mon, Aug 3, 2020 at 11:53 PM Martin Schiller wrote:
>
> I don't like the idea to get rid of the 1-byte header.
> This header is also used in userspace, for example when using a tun/tap
> interface for an XoT (X.25 over TCP) application. A change would
> therefore have very far-reaching conseque
Thanks!
On Mon, Aug 3, 2020 at 2:50 AM Willem de Bruijn
wrote:
>
> It's [PATCH net v3], not [net v3]
Sorry. My mistake. I'll pay attention next time.
I'm currently thinking about changing the subject to reflect that we
added a "skb->len" check. Should I number the new patch as v1 or
continue to
98] packet_sendmsg+0xbf0/0x19b0
..
Additional change:
When sending, check skb->len to ensure the 1-byte pseudo header is
present before reading it.
Cc: Willem de Bruijn
Cc: Brian Norris
Signed-off-by: Xie He
---
Change from v2:
Added skb->len check when sending.
Change from v1:
N
On Sat, Aug 1, 2020 at 6:31 AM Willem de Bruijn
wrote:
>
> The kernel interface cannot be changed. If packet sockets used to pass
> the first byte up to userspace, they have to continue to do so.
>
> So I think you can limit the header_ops to only dev_hard_header.
Actually if we want to keep the
On Fri, Jul 31, 2020 at 7:33 PM Willem de Bruijn
wrote:
>
> I quickly scanned the main x.25 datapath code. Specifically
> x25_establish_link, x25_terminate_link and x25_send_frame. These all
> write this 1 byte header. It appears to be an in-band communication
> means between the network and data
201 - 300 of 323 matches
Mail list logo
