New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LWIP PBUF_POOL_BUFSIZE increased to fit also IPv6 header #6139

Merged
merged 3 commits into from Feb 21, 2018

Conversation

Projects
None yet
5 participants
@VeijoPesonen
Contributor

VeijoPesonen commented Feb 20, 2018

IPv6 header requires 20 more bytes compared to IPv4 header.

Description

To avoid using multiple buffers increase LWIP PBUF_POOL_BUFSIZE. This change increases the default size(IPv4) by 20 bytes required by IPv6 header.

Fixed a Greentea test case WIFI-CONNECT-PARAMS-CHANNEL-FAIL on the side. To make the test case to succeed invalid channel number must be used instead of valid one.

Status

READY

Migrations

If this PR changes any APIs or behaviors, give a short description of what API users should do when this PR is merged.

NO

Related PRs

List related PRs against other branches:

branch PR

Todos

Deploy notes

Notes regarding the deployment of this PR. These should note any required changes in the build environment, tools, compilers and so on.

Steps to test or reproduce

Outline the steps to test or reproduce the PR here.

@VeijoPesonen

This comment has been minimized.

Contributor

VeijoPesonen commented Feb 20, 2018

@kjbracey-arm, @mikaleppanen, please review

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Feb 20, 2018

That's lwip core code - I'd rather we didn't mess with it locally. So should we pull the equivalent logic into our lwipopts.h - effectively take over and not rely on its default?

As this stands it would make sense as a patch to submit to lwIP though...

@0xc0170 0xc0170 requested review from mikaleppanen and kjbracey-arm Feb 20, 2018

@VeijoPesonen VeijoPesonen force-pushed the VeijoPesonen:eth_n_wifi-drv_n_tc_fixes branch from 807e752 to c7e0118 Feb 20, 2018

@VeijoPesonen

This comment has been minimized.

Contributor

VeijoPesonen commented Feb 20, 2018

@kjbracey-arm , @mikaleppanen, changes moved outside LWIP. Lets look for that upstreaming later.

LWIP PBUF_POOL_BUFSIZE increased to fit also IPv6 header
IPv6 header requires 20 more bytes compared to IPv4 header.

@VeijoPesonen VeijoPesonen force-pushed the VeijoPesonen:eth_n_wifi-drv_n_tc_fixes branch from c7e0118 to e1d9d87 Feb 20, 2018

@@ -150,6 +150,12 @@
#ifdef MBED_CONF_LWIP_PBUF_POOL_BUFSIZE
#undef PBUF_POOL_BUFSIZE

This comment has been minimized.

@kjbracey-arm

kjbracey-arm Feb 20, 2018

Contributor

The undef wants to come out of the ifdef, as you're now explicitly setting it in all cases.

This comment has been minimized.

@VeijoPesonen

VeijoPesonen Feb 21, 2018

Contributor

A fix provided. I think I grasped what you were looking for here. Each driver needs to be able to set the PBUF_POOL_BUFSIZE explicitly. In the same manner as PBUF_POOL_SIZE is set.

This comment has been minimized.

@kjbracey-arm

kjbracey-arm Feb 21, 2018

Contributor

Ah, that wasn't actually what I meant, but you're right, that's consistent.

@@ -150,6 +150,12 @@
#ifdef MBED_CONF_LWIP_PBUF_POOL_BUFSIZE
#undef PBUF_POOL_BUFSIZE
#define PBUF_POOL_BUFSIZE MBED_CONF_LWIP_PBUF_POOL_BUFSIZE
#else

This comment has been minimized.

@kjbracey-arm

kjbracey-arm Feb 20, 2018

Contributor

Could also use #elif here. Not sure if it's better or worse, but consider it.

This comment has been minimized.

@VeijoPesonen

VeijoPesonen Feb 21, 2018

Contributor

Duly noted.

VeijoPesonen added some commits Feb 20, 2018

Fix WIFI-CONNECT-PARAMS-CHANNEL-FAIL Greentea test case
The test case is executed succesfully when usage of wrong channel is detected
@0xc0170

This comment has been minimized.

Member

0xc0170 commented Feb 21, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Feb 21, 2018

Build : SUCCESS

Build number : 1193
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6139/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@@ -150,6 +150,12 @@
#ifdef MBED_CONF_LWIP_PBUF_POOL_BUFSIZE
#undef PBUF_POOL_BUFSIZE

This comment has been minimized.

@kjbracey-arm

kjbracey-arm Feb 21, 2018

Contributor

Ah, that wasn't actually what I meant, but you're right, that's consistent.

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Feb 21, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Feb 21, 2018

/morph uvisor-test

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 merged commit 5c7cd1f into ARMmbed:master Feb 21, 2018

18 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
mbed-ci-generic Build finished.
Details
travis-ci/docs/ Local docs testing has passed
Details
travis-ci/events/ Local events testing has passed
Details
travis-ci/littlefs/ Local littlefs testing has passed
Details
travis-ci/mbed2-ATMEL/ Local mbed2-ATMEL testing has passed
Details
travis-ci/mbed2-MAXIM/ Local mbed2-MAXIM testing has passed
Details
travis-ci/mbed2-NORDIC/ Local mbed2-NORDIC testing has passed
Details
travis-ci/mbed2-NUVOTON/ Local mbed2-NUVOTON testing has passed
Details
travis-ci/mbed2-NXP/ Local mbed2-NXP testing has passed
Details
travis-ci/mbed2-SILICON_LABS/ Local mbed2-SILICON_LABS testing has passed
Details
travis-ci/mbed2-STM/ Local mbed2-STM testing has passed
Details

@VeijoPesonen VeijoPesonen deleted the VeijoPesonen:eth_n_wifi-drv_n_tc_fixes branch Feb 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment